Skip to content

Conversation

suratkhan
Copy link
Collaborator

@suratkhan suratkhan commented Sep 24, 2025

  • Added 10 vulnerable smart contracts
  • Vulnerability type: Integer Overflow

Summary by CodeRabbit

  • New Features
    • Added a suite of sample smart contracts to expand coverage of integer-overflow and unsafe-transfer scenarios. Includes cases for token sale finalization, piggy-bank withdrawals, holder payouts, simple bank withdrawals, token transfers, private bank cash-out, basic token transfer, timed claims, nested token balance withdrawals, and coin purchases. These introduce public functions and states to better surface arithmetic under/overflow, reentrancy, and low-level call risks, improving audit coverage and educational examples.

@suratkhan suratkhan requested a review from aleskrin September 24, 2025 11:59
Copy link

coderabbitai bot commented Sep 24, 2025

Walkthrough

Adds 10 new Solidity contracts under audit_engine/smart_contracts/integer_overflow, each introducing public state and functions related to token balances, withdrawals, sales, or claims, many using low-level Ether transfers via call.value(...). Functions include finish, Collect, WithdrawToHolder, withdraw, eT, CashOut, transfer, claim, withdraw, and buyRecipient.

Changes

Cohort / File(s) Summary
Low-level withdrawal via call.value(...)
audit_engine/.../io52.sol, audit_engine/.../io53.sol, audit_engine/.../io54.sol, audit_engine/.../io56.sol, audit_engine/.../io59.sol
Add contracts with public balance mappings and withdrawal-like functions that send Ether using low-level calls and adjust balances (Collect, WithdrawToHolder, withdraw, CashOut, withdraw).
Arithmetic/transfer logic without safeguards
audit_engine/.../io55.sol, audit_engine/.../io57.sol, audit_engine/.../io58.sol
Add contracts with token/balance operations lacking checks: eT transfer and refund, BasicToken.transfer subtracts balance, OysterPearl.claim subtracts claimAmount after timestamp check.
Sale/allocation and fund forwarding
audit_engine/.../io51.sol, audit_engine/.../io60.sol
Add contracts that compute allocations and forward funds: AuctusTokenSale.finish uses address(this).call.value(...), MoldCoin.buyRecipient mints tokens and forwards msg.value to founder.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant Contract as Contract (withdraw-like)
  participant UserWallet as User Wallet

  User->>Contract: withdraw/Collect/CashOut(amount)
  alt Balance check present
    Contract->>Contract: Update internal balance
  else No balance check / deferred update
    Note over Contract: State unchanged before call
  end
  Contract->>UserWallet: low-level call.value(amount)()
  alt Call success
    Contract->>Contract: Adjust balance (if not yet)
    Contract-->>User: return true/complete
  else Call failure
    Contract-->>User: revert/return false
  end
Loading
sequenceDiagram
  autonumber
  actor Buyer
  participant MoldCoin
  participant Founder as Founder Address

  Buyer->>MoldCoin: buyRecipient(recipient) with msg.value
  MoldCoin->>MoldCoin: tokens = msg.value * block.timestamp
  MoldCoin->>MoldCoin: balances[recipient] += tokens
  MoldCoin->>MoldCoin: amountRaised += msg.value
  MoldCoin->>Founder: call.value(msg.value)()
  alt success
    MoldCoin-->>Buyer: complete
  else failure
    MoldCoin-->>Buyer: revert
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested reviewers

  • aleskrin
  • robaribas

Poem

In burrows of bytes, I thump with delight,
New contracts sprout in Solidity’s night.
Calls trickle, tokens tick, clocks chime with might—
Careful, dear devs, on arithmetic’s bite!
I twitch my ears, review in sight—
Hop, hop, hooray, the audits take flight! 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title accurately describes the primary change: adding ten smart contracts that demonstrate integer overflow vulnerabilities and corresponds to the files listed in the PR (io51–io60), so it is relevant and specific enough for history scanning; however, it contains a typographical error ("inteeger") and slightly awkward phrasing.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/smart-cntrcts-int-over-7

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 28

🧹 Nitpick comments (22)
audit_engine/smart_contracts/integer_overflow/io58.sol (2)

6-6: Meaningless timestamp check (always true).

block.timestamp >= 60 will be true for all practical deployments. Either remove it or gate against a start variable.

Apply this to remove:

-        require(block.timestamp >= 60);

2-3: No way to set claimAmount or seed balances — claim() is a no‑op by default.

Without a setter/deposit, claimAmount defaults to 0 and balanceOf is 0 for all, so the vulnerability isn’t practically exercisable.

If you want this contract to be self‑contained for demos, add minimal setters (no access control, since this is a vuln sample):

     mapping (address => uint256) public balanceOf;
 
+    // Demo helpers to exercise the vuln
+    function setClaimAmount(uint256 _a) public { claimAmount = _a; }
+    function seedBalance(address who, uint256 amt) public { balanceOf[who] = amt; }
+
     function claim() public {
audit_engine/smart_contracts/integer_overflow/io56.sol (2)

6-6: Add explicit visibility.

Solidity >=0.5 requires it; being explicit improves clarity even on 0.4.x.

-    function CashOut(uint _am) {
+    function CashOut(uint _am) public {

4-6: Optional: enable funding path or document how the contract is funded.

There’s no payable entrypoint; consider adding a payable fallback for local testing/demos.

     mapping (address => uint) public balances;
 
-    function CashOut(uint _am) {
+    function() public payable {}
+
+    function CashOut(uint _am) {

Is this contract intentionally left unfunded (e.g., funded via selfdestruct in tests), or should we include a simple deposit/fallback?

audit_engine/smart_contracts/integer_overflow/io52.sol (3)

7-12: Reentrancy and unchecked call result — confirm if multi‑vuln is intended.

  • External call occurs before state update (classic reentrancy).
  • Return value from call is ignored.
    If you want this sample to focus solely on integer underflow/overflow, consider moving the state update before the call (still keeps the underflow) and/or documenting the intent.

Optional tweak to de‑emphasize reentrancy while keeping the underflow:

-        if(Bal[msg.sender] >= MinSum) {
-            msg.sender.call.value(_am)();
-            Bal[msg.sender] -= _am;
-        }
+        if (Bal[msg.sender] >= MinSum) {
+            Bal[msg.sender] -= _am;           // underflow remains if _am > Bal[msg.sender]
+            msg.sender.call.value(_am)();     // external interaction after effects
+        }

4-12: No code path populates Bal — branch likely unreachable.

There’s no deposit/credit function, so Bal[msg.sender] >= MinSum will be false for all users in normal execution. If static analysis only is intended, fine; otherwise add a minimal deposit.

Example (0.4.x):

function deposit() public payable {
    Bal[msg.sender] += msg.value; // intentionally unchecked to stay in the “integer overflow” family
}

Want me to add this here and in the other samples for consistency?


7-7: Collect doesn’t use msg.value; payable is unnecessary.

If you add a separate deposit function, you can drop payable here.

-    function Collect(uint _am) public payable {
+    function Collect(uint _am) public {
audit_engine/smart_contracts/integer_overflow/io59.sol (3)

8-8: Low-level call.value(...)() is reentrancy-prone; avoid unless you need it explicitly

Using call.value forwards all gas and can enable reentrancy. Your state update helps, but combined with the underflow it’s exploitable. Prefer transfer (for 0.4.x) or a pull pattern with a reentrancy guard.


4-4: Nested mapping but only address(0) used — consider simplifying

If this is ETH-only, a single mapping mapping(address => uint) balances; is clearer. If the first key is meant to be a token address, use address(0) consistently for ETH.


6-6: Make visibility explicit

Solidity 0.4.x defaults to public, but being explicit improves readability and future-compatibility.

-function withdraw(uint amount) {
+function withdraw(uint amount) public {
audit_engine/smart_contracts/integer_overflow/io57.sol (2)

4-7: Non‑ERC20‑like transfer signature and no event

If the goal is to resemble real‑world vulnerable ERC‑20 patterns, consider adding a recipient and emitting an event; it also makes the underflow impact observable.

Possible shape (still intentionally vulnerable if you omit the require):

-    function transfer(uint256 _value) public returns (bool) {
-        balances[msg.sender] = balances[msg.sender] - _value;
-        return true;
-    }
+    event Transfer(address indexed from, address indexed to, uint256 value);
+    function transfer(address _to, uint256 _value) public returns (bool) {
+        // Intentionally vulnerable if you remove this check
+        // require(balances[msg.sender] >= _value, "insufficient balance");
+        unchecked { balances[msg.sender] = balances[msg.sender] - _value; }
+        balances[_to] = balances[_to] + _value;
+        emit Transfer(msg.sender, _to, _value);
+        return true;
+    }

2-2: No mint/init path; exploit isn’t easily demonstrable without a sink

With only a self‑balance decrement, the underflow is visible but not actionable. If these samples feed detectors/tests, consider adding a simple mint or a withdraw/transfer‑to‑recipient path to observe consequences.

I can add a minimal “mint” function or a second contract that reads/spends these balances for clearer test assertions.

audit_engine/smart_contracts/integer_overflow/io54.sol (1)

4-4: Prefer uint256 and optionally expose a getter.

Align with Solidity conventions and EVM word size:

-    mapping (address => uint) accountBalances;
+    mapping(address => uint256) public accountBalances;

[Suggest making it public only if external visibility is useful for your tests.]

audit_engine/smart_contracts/integer_overflow/io55.sol (3)

4-4: AI summary inconsistency: balances is not public

The summary says “public balances mapping,” but the code declares it without visibility, so it’s internal. If you intended public getters, mark it public.


6-11: Ambiguous token↔Ether coupling and missing events

Using _tkA both as token amount and wei is brittle. Clarify units or decouple logic. Also, emit events for observability.

  • Consider renaming and documenting units, or separating token transfer from Ether payout.
  • Add events:
event Transfer(address indexed from, address indexed to, uint256 value);
event Payout(address indexed to, uint256 weiAmount);

Emit them on state changes and payouts.


6-6: Non-descriptive function name

eT is opaque. Prefer a descriptive name (e.g., transferAndPayout) for maintainability and audits.

audit_engine/smart_contracts/integer_overflow/io60.sol (6)

7-7: Expose balances via getter (optional)

Helps audits and tests query balances without extra functions.

-    mapping(address => uint) balances;
+    mapping(address => uint) public balances;

9-9: Be explicit about visibility

Old compilers assume public, but being explicit avoids ambiguity.

-    function buyRecipient(address recipient) payable {
+    function buyRecipient(address recipient) public payable {

9-10: Timestamp-dependent minting is miner-influenced

block.timestamp can be skewed within bounds; it also grows over time, inflating later buyers.

Consider a fixed rate or price function decoupled from timestamp.


9-9: Validate recipient != address(0)

Prevents accidental credits to the zero address.

     function buyRecipient(address recipient) public payable {
+        require(recipient != address(0));

5-5: Unused coinAllocation

Either enforce a cap or drop the variable.

For example:

-    uint public coinAllocation = 20 * 10**8 * 10**2;
+    uint public coinAllocation = 20 * 10**8 * 10**2; // total cap

Then guard mints against exceeding coinAllocation.


9-14: Emit an event for purchases

Improves traceability in tests/audits.

+    event Purchase(address indexed buyer, address indexed recipient, uint value, uint tokens);
     function buyRecipient(address recipient) public payable {
         uint tokens = msg.value * block.timestamp;
         balances[recipient] = balances[recipient] + tokens;
         amountRaised = amountRaised + msg.value;
         founder.transfer(msg.value);
+        emit Purchase(msg.sender, recipient, msg.value, tokens);
     }
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 16cd3d9 and 2996c80.

📒 Files selected for processing (10)
  • audit_engine/smart_contracts/integer_overflow/io51.sol (1 hunks)
  • audit_engine/smart_contracts/integer_overflow/io52.sol (1 hunks)
  • audit_engine/smart_contracts/integer_overflow/io53.sol (1 hunks)
  • audit_engine/smart_contracts/integer_overflow/io54.sol (1 hunks)
  • audit_engine/smart_contracts/integer_overflow/io55.sol (1 hunks)
  • audit_engine/smart_contracts/integer_overflow/io56.sol (1 hunks)
  • audit_engine/smart_contracts/integer_overflow/io57.sol (1 hunks)
  • audit_engine/smart_contracts/integer_overflow/io58.sol (1 hunks)
  • audit_engine/smart_contracts/integer_overflow/io59.sol (1 hunks)
  • audit_engine/smart_contracts/integer_overflow/io60.sol (1 hunks)
🔇 Additional comments (7)
audit_engine/smart_contracts/integer_overflow/io58.sol (1)

7-7: Underflow on subtraction (intentional?) — clarify via pragma or comment.

This line underflows for Solidity <0.8. If that’s the goal, great—please make it explicit via pragma or a comment. If targeting >=0.8, use unchecked to demonstrate the issue.

  • Option A (paired with pre‑0.8 pragma above): no change needed.
  • Option B (Solidity >=0.8):
-        balanceOf[msg.sender] -= claimAmount;
+        unchecked { balanceOf[msg.sender] -= claimAmount; }

Consider adding an inline note:

+        // VULN: unchecked subtraction can underflow and wrap (demonstration)
audit_engine/smart_contracts/integer_overflow/io59.sol (1)

6-7: Integer underflow (not overflow) — align dataset/PR labeling

  • withdraw() subtracts amount from tokens[0][msg.sender] → underflow risk; PR title says "Integer Overflow" — rename or make dataset consistently "Overflow/Underflow" across the 10 contracts.
  • Verification: ran the supplied checks but the search returned no matches (no output), so pragma pinning and other underflow patterns couldn't be confirmed. Re-run the verification script from the repository root and paste its output or provide the correct path.
audit_engine/smart_contracts/integer_overflow/io53.sol (1)

2-13: Verify testing approach for vulnerable contracts

Since these contracts are intentionally vulnerable for testing purposes, ensure they are:

  1. Clearly marked as vulnerable test contracts
  2. Never deployed to mainnet
  3. Stored in a clearly labeled test directory
  4. Have corresponding non-vulnerable versions for comparison

Consider adding clear warning comments:

+/**
+ * @title VULNERABLE CONTRACT - FOR TESTING ONLY
+ * @dev Contains intentional integer overflow vulnerabilities
+ * @dev DO NOT DEPLOY TO MAINNET
+ */
 contract TokenBank  {
audit_engine/smart_contracts/integer_overflow/io57.sol (2)

4-7: Arithmetic underflow in transfer — add balance check or confirm compiler behavior

balances[msg.sender] is decremented without a guard; on Solidity <0.8 this underflows (wraps to 2^256−1). Either add a require check or confirm the repo uses Solidity >=0.8 (where arithmetic reverts) and annotate intent.

Location: audit_engine/smart_contracts/integer_overflow/io57.sol (lines 4–7)

Suggested fix:

     function transfer(uint256 _value) public returns (bool) {
-        balances[msg.sender] = balances[msg.sender] - _value;
+        require(balances[msg.sender] >= _value, "insufficient balance");
+        balances[msg.sender] = balances[msg.sender] - _value;
         return true;
     }

1-8: Confirm pragma/unchecked consistency in integer_overflow contracts

Verification script found no .sol files or any pragma solidity/unchecked occurrences under audit_engine/smart_contracts/integer_overflow, so consistency couldn't be verified — ensure each contract either pins a pre-0.8 pragma or, if using >=0.8, explicitly uses unchecked where overflow is intended; rerun the check or point to the actual .sol files for re-verification.

audit_engine/smart_contracts/integer_overflow/io55.sol (1)

2-12: Confirm vulnerable samples are excluded from build/deploy/test pipelines

No standard Solidity build configs or CI workflows were found in the repo (no package.json, hardhat/foundry/truffle/brownie configs, or .github/workflows). This is inconclusive — verify CI/build scripts, Makefile, Dockerfile, and any external pipelines explicitly exclude audit_engine/smart_contracts/integer_overflow (e.g., audit_engine/smart_contracts/integer_overflow/io55.sol) from production builds and testnets.

audit_engine/smart_contracts/integer_overflow/io60.sol (1)

9-12: Integer overflow in token minting and additions — confirm intentional for this dataset

msg.value * block.timestamp, balances[recipient] + tokens, and amountRaised + msg.value can overflow in 0.4.x.

Optionally annotate for clarity:

     function buyRecipient(address recipient) public payable {
+        // VULN: integer overflow by design (test fixture).
         uint tokens = msg.value * block.timestamp;

If not intentional, switch to 0.8.x or use SafeMath.

@@ -0,0 +1,9 @@

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Missing pragma directive and compiler version

The contract lacks a Solidity version pragma, making it vulnerable to compilation with different compiler versions that may have different overflow behaviors.

+pragma solidity ^0.8.0; // Or specify the intended vulnerable version
+
 contract AuctusTokenSale {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
pragma solidity ^0.8.0; // Or specify the intended vulnerable version
contract AuctusTokenSale {
🤖 Prompt for AI Agents
In audit_engine/smart_contracts/integer_overflow/io51.sol around lines 1 to 1
the file is missing a Solidity pragma directive; add a top-of-file pragma such
as "pragma solidity ^0.8.0;" (or a pinned range like ">=0.8.0 <0.9.0") to ensure
the compiler uses a version with built-in overflow/underflow checks and to avoid
accidental compilation with older compilers.

Comment on lines +4 to +8
function finish() public {
uint256 freeEthers = address(this).balance * 40;
uint256 vestedEthers = address(this).balance - freeEthers;
assert(address(this).call.value(vestedEthers)());
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Critical: Integer overflow vulnerability in balance calculation

The multiplication address(this).balance * 40 on line 5 can overflow, causing freeEthers to wrap around to a small value. This would result in vestedEthers being calculated incorrectly on line 6, potentially draining the entire contract balance.

Example overflow scenario:

  • If address(this).balance = 2^256 / 40, then freeEthers would overflow
  • The subtraction on line 6 would then underflow since freeEthers (after overflow) would be less than address(this).balance
 function finish() public  {
-    uint256 freeEthers = address(this).balance * 40;
-    uint256 vestedEthers = address(this).balance - freeEthers;
+    // Use SafeMath or Solidity 0.8+ with automatic overflow checks
+    uint256 balance = address(this).balance;
+    require(balance <= type(uint256).max / 40, "Overflow protection");
+    uint256 freeEthers = balance * 40;
+    require(freeEthers <= balance, "Invalid calculation");
+    uint256 vestedEthers = balance - freeEthers;
     assert(address(this).call.value(vestedEthers)());
 }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In audit_engine/smart_contracts/integer_overflow/io51.sol around lines 4 to 8,
the expression uint256 freeEthers = address(this).balance * 40 can overflow;
change the arithmetic to be overflow-safe and ensure vestedEthers calculation is
correct — either use a checked math library (SafeMath) for mul/sub and require
no overflow/underflow, or rearrange to avoid large intermediate values (e.g.,
compute freeEthers = address(this).balance / 100 * 40 to perform the division
before multiplication), then compute vestedEthers = address(this).balance -
freeEthers using the same checked arithmetic and add a require that vestedEthers
<= address(this).balance to prevent underflow.


function finish() public {
uint256 freeEthers = address(this).balance * 40;
uint256 vestedEthers = address(this).balance - freeEthers;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Critical: Integer underflow leads to massive vestedEthers value

Since freeEthers = address(this).balance * 40, this will always be 40 times larger than the balance (unless overflow occurs). The subtraction address(this).balance - freeEthers will underflow, wrapping around to a huge value close to 2^256.

This is the core integer overflow vulnerability - the calculation is mathematically impossible without underflow.

function finish() public {
uint256 freeEthers = address(this).balance * 40;
uint256 vestedEthers = address(this).balance - freeEthers;
assert(address(this).call.value(vestedEthers)());
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Critical: Self-transfer with underflowed value will fail

The contract attempts to send vestedEthers (an underflowed huge value) to itself via address(this).call.value(vestedEthers)(). This will fail because the contract cannot send more than its balance, causing the assert to fail and revert the transaction.

Additionally:

  • Using low-level call for Ether transfers is discouraged
  • Sending Ether to the contract itself serves no purpose
  • Missing access control allows anyone to trigger this function
🤖 Prompt for AI Agents
In audit_engine/smart_contracts/integer_overflow/io51.sol around line 7, the
code performs a low-level self-transfer using
address(this).call.value(vestedEthers)() with an underflowed vestedEthers which
will always fail and revert; remove the pointless self-send and the assert, add
a proper balance check (require(vestedEthers <= address(this).balance)) if funds
should be moved, and if the intention is to pay out to an external beneficiary
replace the low-level call with a safe Ether transfer pattern (e.g., transfer to
a designated beneficiary address or pull-payment pattern) and add proper access
control (onlyOwner or a modifier) so arbitrary callers cannot trigger the
payout.

@@ -0,0 +1,13 @@

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

🧩 Analysis chain

Add SPDX and an explicit pragma (<0.8) to preserve the overflow behavior.

Without a pragma, builds can be non‑deterministic. Also, if compiled on 0.8+, arithmetic underflow will revert and the intended “integer overflow” test case won’t manifest.

Apply:

+// SPDX-License-Identifier: UNLICENSED
+pragma solidity ^0.4.24;
 
 contract SIMPLE_PIGGY_BANK {

🏁 Script executed:

#!/bin/bash
set -euo pipefail
echo "---- file: audit_engine/smart_contracts/integer_overflow/io52.sol ----"
sed -n '1,240p' audit_engine/smart_contracts/integer_overflow/io52.sol || true
echo "---- end ----"

Length of output: 374


Add SPDX header and explicit pre-0.8 pragma to preserve overflow behavior

Add to the top of audit_engine/smart_contracts/integer_overflow/io52.sol so builds are deterministic and arithmetic overflow behavior remains pre-0.8:

+// SPDX-License-Identifier: UNLICENSED
+pragma solidity ^0.4.24;
 
 contract SIMPLE_PIGGY_BANK {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.4.24;
contract SIMPLE_PIGGY_BANK {
🤖 Prompt for AI Agents
In audit_engine/smart_contracts/integer_overflow/io52.sol at lines 1-1, the file
is missing an SPDX license identifier and an explicit pre-0.8 Solidity pragma;
add a top-of-file SPDX header (for example "SPDX-License-Identifier: MIT")
followed by an explicit pragma specifying a Solidity compiler version below 0.8
(for example "pragma solidity ^0.7.0;") so builds are deterministic and
arithmetic overflow behavior remains pre-0.8.

Comment on lines +6 to +9
function withdraw(uint amount) {
tokens[0][msg.sender] = tokens[0][msg.sender] - amount;
require(msg.sender.call.value(amount)());
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Integer underflow on balance subtraction (allows draining)

tokens[0][msg.sender] - amount underflows if amount > balance, wrapping to a huge number. Also uses literal 0 for an address key and lacks basic input checks.

Apply checks and use address(0); also make visibility explicit and avoid call.value:

-function withdraw(uint amount) {
-    tokens[0][msg.sender] = tokens[0][msg.sender] - amount;
-    require(msg.sender.call.value(amount)());
-}
+function withdraw(uint amount) public {
+    require(amount > 0);
+    uint balance = tokens[address(0)][msg.sender];
+    require(balance >= amount);
+    tokens[address(0)][msg.sender] = balance - amount;
+    msg.sender.transfer(amount);
+}

Note: If this contract is intentionally vulnerable for test fixtures, keep the underflow as-is but consider the comments below to avoid confounding with other vuln types.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In audit_engine/smart_contracts/integer_overflow/io59.sol around lines 6-9, the
withdraw function subtracts amount from a balance without checking bounds, uses
a literal 0 as an address key, lacks explicit visibility, and uses call.value
(reentrant-prone). Fix by: (1) make the function visibility explicit (e.g.,
public/external); (2) validate inputs with requires (e.g., amount > 0 and amount
<= current balance) to prevent underflow; (3) use the correct address key
(replace the literal 0 with address(0) or the appropriate mapping key) when
indexing balances; (4) update the caller's balance before transferring funds and
use a safer transfer pattern (e.g., address.transfer or a pull-payment pattern /
reentrancy guard) instead of raw call.value. Ensure all changes preserve
existing storage layout and tests.

Comment on lines +1 to +3

contract MoldCoin {

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

🧩 Analysis chain

Pin Solidity version to preserve intended semantics and avoid build breaks

Without a pragma, toolchains may compile with 0.8+, breaking call.value and eliminating overflow behavior.

Apply:

+pragma solidity 0.4.26;
 
 contract MoldCoin {

🏁 Script executed:

#!/bin/bash
set -euo pipefail
echo "Showing top of file if it exists:"
if [ -f "audit_engine/smart_contracts/integer_overflow/io60.sol" ]; then
  sed -n '1,120p' audit_engine/smart_contracts/integer_overflow/io60.sol || true
else
  echo "File not found: audit_engine/smart_contracts/integer_overflow/io60.sol"
  rg -n "io60.sol" || true
fi

Length of output: 494


Add explicit Solidity pragma to pin compiler to 0.4.26

File lacks a pragma; without one modern compilers (0.8+) change overflow semantics and break old call.value usage — pin to 0.4.26.

+pragma solidity 0.4.26;
 
 contract MoldCoin {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
contract MoldCoin {
pragma solidity 0.4.26;
contract MoldCoin {
🤖 Prompt for AI Agents
In audit_engine/smart_contracts/integer_overflow/io60.sol around lines 1 to 3,
the file is missing a Solidity pragma and must be pinned to the 0.4.26 compiler;
add a version pragma declaration (e.g. pragma solidity 0.4.26;) as the first
non-comment line in the file, before the contract declaration so the code
compiles with the expected pre-0.5 overflow and call.value semantics.

Comment on lines +1 to +15

contract MoldCoin {

address public founder;
uint public coinAllocation = 20 * 10**8 * 10**2;
uint public amountRaised = 0;
mapping(address => uint) balances;

function buyRecipient(address recipient) payable {
uint tokens = msg.value * block.timestamp;
balances[recipient] = balances[recipient] + tokens;
amountRaised = amountRaised + msg.value;
if (!founder.call.value(msg.value)()) revert();
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

🧩 Analysis chain

Quick repo check: ensure all new overflow fixtures compile under the intended compiler

Verify all newly added contracts in this folder declare a 0.4.x pragma (or, if using 0.8.x intentionally, update syntax and wrap arithmetic in unchecked).


🏁 Script executed:

#!/bin/bash
# Verify solidity pragmas and deprecated patterns across the new fixtures.
echo "Files without a pragma:"
rg -nP --type=sol '^\s*pragma\s+solidity' -C0 | sed 's/:.*//' | sort -u > /tmp/has_pragma.txt
fd -e sol audit_engine/smart_contracts/integer_overflow | sort -u | comm -23 - /tmp/has_pragma.txt || true

echo -e "\nOccurrences of deprecated call.value syntax (0.6+ incompatible):"
rg -nP --type=sol '\.call\.value\s*\(' -C2 audit_engine/smart_contracts/integer_overflow

echo -e "\nSolidity versions used:"
rg -nP --type=sol '^\s*pragma\s+solidity' -C0 audit_engine/smart_contracts/integer_overflow | sort -u

Length of output: 684


🏁 Script executed:

#!/bin/bash
set -euo pipefail
SOLDIR='audit_engine/smart_contracts/integer_overflow'
echo "Searching for .sol files under $SOLDIR"
files=$(find "$SOLDIR" -type f -name '*.sol' || true)
if [ -z "$files" ]; then
  echo "No .sol files found"
  exit 0
fi
for f in $files; do
  echo "---- $f ----"
  echo "Pragma lines:"
  grep -nE '^\s*pragma\s+solidity' "$f" || echo "  NONE"
  echo "Deprecated .call.value occurrences:"
  grep -n '\.call\.value' "$f" || echo "  NONE"
  echo "First 15 lines:"
  sed -n '1,15p' "$f" || true
  echo
done

Length of output: 27592


Quick repo check: add Solidity pragmas and fix deprecated .call.value usages in integer_overflow fixtures

  • All .sol files under audit_engine/smart_contracts/integer_overflow are missing a pragma solidity declaration — add an explicit pragma matching the intended compiler (e.g. ^0.4.x) or migrate to ^0.8.x and adapt code/overflow handling.
  • Files with 0.6+‑incompatible .call.value usages (update these to call{value: ...}(...), transfer/send, or otherwise fix reentrancy/return-value handling):
    • audit_engine/smart_contracts/integer_overflow/io25.sol:8 — require(msg.sender.call.value(amountRaised)());
    • audit_engine/smart_contracts/integer_overflow/io28.sol:10 — assert(msg.sender.call.value(_value)());
    • audit_engine/smart_contracts/integer_overflow/io29.sol:7 — require(_exchange.call.value(_value)(_data));
    • audit_engine/smart_contracts/integer_overflow/io43.sol:12 — if(!giveEthCharityAddress.call.value(ethToPay).gas(400000)()) {
    • audit_engine/smart_contracts/integer_overflow/io45.sol:10 — require(msg.sender.call.value(amount)());
    • audit_engine/smart_contracts/integer_overflow/io48.sol:8 — require(_contract.call.value(_amount)());
    • audit_engine/smart_contracts/integer_overflow/io49.sol:9 — msg.sender.call.value(_value)(_data);
    • audit_engine/smart_contracts/integer_overflow/io51.sol:7 — assert(address(this).call.value(vestedEthers)());
    • audit_engine/smart_contracts/integer_overflow/io52.sol:9 — msg.sender.call.value(_am);
    • audit_engine/smart_contracts/integer_overflow/io53.sol:8 — if(_addr.call.value(_wei)()) {
    • audit_engine/smart_contracts/integer_overflow/io54.sol:8 — msg.sender.call.value(amount);
    • audit_engine/smart_contracts/integer_overflow/io55.sol:9 — if (!msg.sender.call.value(_tkA)()) revert();
    • audit_engine/smart_contracts/integer_overflow/io56.sol:7 — if(msg.sender.call.value(_am)()){
    • audit_engine/smart_contracts/integer_overflow/io59.sol:8 — require(msg.sender.call.value(amount)());
    • audit_engine/smart_contracts/integer_overflow/io60.sol:13 — if (!founder.call.value(msg.value)()) revert();
  • Action: either add a 0.4.x pragma to these fixtures if they are intended for 0.4, or migrate all files to ^0.8.x (update call syntax, explicit visibilities/constructors, and wrap intentional overflows in unchecked) before running compilation/tests.

Comment on lines +4 to +7
address public founder;
uint public coinAllocation = 20 * 10**8 * 10**2;
uint public amountRaised = 0;
mapping(address => uint) balances;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Founder is never initialized — ETH will be forwarded to address(0)

Forwarding to the zero address effectively burns funds.

Apply:

 contract MoldCoin {
      
     address public founder;
     uint public coinAllocation = 20 * 10**8 * 10**2;
     uint public amountRaised = 0;
     mapping(address => uint) balances;
 
+    constructor() public {
+        founder = msg.sender;
+    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
address public founder;
uint public coinAllocation = 20 * 10**8 * 10**2;
uint public amountRaised = 0;
mapping(address => uint) balances;
address public founder;
uint public coinAllocation = 20 * 10**8 * 10**2;
uint public amountRaised = 0;
mapping(address => uint) balances;
constructor() public {
founder = msg.sender;
}
🤖 Prompt for AI Agents
In audit_engine/smart_contracts/integer_overflow/io60.sol around lines 4 to 7,
the founder state variable is never initialized so any ETH forwarded to it will
go to address(0); initialize founder (e.g., set founder = msg.sender) during
contract deployment by assigning it in the constructor (or at declaration if you
want a fixed address), and add a require(founder != address(0)) guard where
funds are forwarded to ensure you never send ETH to the zero address.

uint tokens = msg.value * block.timestamp;
balances[recipient] = balances[recipient] + tokens;
amountRaised = amountRaised + msg.value;
if (!founder.call.value(msg.value)()) revert();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Avoid low-level call.value; use transfer in 0.4.x to reduce reentrancy/gas pitfalls

call.value forwards all gas and complicates error handling.

-        if (!founder.call.value(msg.value)()) revert();
+        founder.transfer(msg.value);

If upgrading to 0.8.x later, use: (bool ok, ) = founder.call{value: msg.value}(""); require(ok);

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (!founder.call.value(msg.value)()) revert();
founder.transfer(msg.value);
🤖 Prompt for AI Agents
In audit_engine/smart_contracts/integer_overflow/io60.sol around line 13, the
code uses low-level call.value which forwards all gas and complicates error
handling; replace it with founder.transfer(msg.value) when compiling under
Solidity 0.4.x so failures automatically revert, and if you intend to upgrade to
0.8.x use the safe pattern (bool ok, ) = founder.call{value: msg.value}("");
require(ok); to explicitly check the send result and avoid reentrancy/gas
issues.

@aleskrin aleskrin merged commit fa5afa4 into main Sep 24, 2025
1 check passed
@robaribas robaribas deleted the feature/smart-cntrcts-int-over-7 branch October 2, 2025 11:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants