AntFleet

Disagreement · d121d4bf-openai-1

Unconditional unlimited ERC20 approve may revert on tokens requiring allowance reset to zero first (e.g., USDT)

mismatch
repo a16d2030·PR #4·reviewed 1 week ago

Primary finding

Unconditional unlimited ERC20 approve may revert on tokens requiring allowance reset to zero first (e.g., USDT)

highbughigh
  • symbiosis/scripts/symbiosis-swap.py:18-19
  • symbiosis/scripts/symbiosis-swap.py:145-156
Many ERC20 tokens (notably USDT on some chains) require setting allowance to 0 before changing it from a non-zero value. This code always tries to set allowance directly to MAX_UINT256 with a single approve call, which will revert on such tokens if an existing non-zero allowance is present. The script will then exit with an error, blocking swaps for affected tokens.

Recommendation

Implement a safe-approve flow: - Check current allowance (allowance(owner, spender)); if < required amount, then: - First call approve(spender, 0) - Then approve(spender, exact required amount) or a reasonable cap. If reading allowance is not available, conservatively send two approvals: approve(spender, 0) followed by approve(spender, amount). Alternatively, integrate Permit/Permit2 when available.

Counterpart finding

Symbiosis API key/credentials and HTTP error responses leak as full JSON to stderr

lowsecuritymedium
  • symbiosis/scripts/symbiosis-swap.py:130-132
  • symbiosis/scripts/symbiosis-swap.py:145-147
  • symbiosis/scripts/symbiosis-swap.py:41-47
Errors from `urllib.request.urlopen` (HTTPError) are not caught and will propagate with a traceback that includes the request URL but generally not auth headers (urllib hides headers in repr). The Bankr response is dumped raw on failure — if the API ever includes wallet/private info or trace data, it ends up in stderr/logs verbatim. Also, `urlopen` raises HTTPError on non-2xx, so the friendly 'ERROR from Symbiosis API' branch is dead code for 4xx/5xx responses (they raise before `'tx' not in result` runs). Lower severity but combines a usability and minor security concern.

Recommendation

Wrap api_post calls in try/except HTTPError and parse response body for `message`; redact known sensitive keys before dumping.

Why this didn't post

This finding didn't meet AntFleet's unanimous agreement threshold. Both frontier models review every PR independently; only findings they both flag with the same severity and category are posted to the PR. This one fell through.

read the methodology →

From the same review

These findings passed the unanimous gate on the same PR review. The disagreement above was filtered out; the findings below were posted.

← back to all disagreementsview public receiptssee unanimous findings + anatomies →
Tweet ↗