fix: resolve DoS for zero-intolerant tokens in approveAndBridge (Fixes #5)

[abiiralbhattarai]

Summary

Resolved a vulnerability where ApproveAndBridge could enter a permanent Denial of Service (DoS) state when interacting with "Zero-Intolerant" tokens that revert on approve(0).

The Issue

When residual "dust" allowance exists, SafeERC20.forceApprove attempts a reset-to-zero before updating. Zero-intolerant tokens revert on this reset, blocking all future bridge attempts for that token.

The Solution

• Sufficient Check: Skips approval if existing dust covers the required balance.
• Direct Attempt: Uses a low-level .call to update allowance , bypassing the revert.
• Safe Fallback: Falls back to forceApprove only if the direct call fails (e.g., for USDT).
• Silent Cleanup: Performs a post-bridge approve(0) via low-level call to ensure no settlement reverts even if the cleanup fails.

Summary by CodeRabbit

• Bug Fixes
  • Improved token approval reliability with a fallback path when standard approval fails.
  • Verified current allowances before applying approvals to avoid unnecessary transactions.
  • Added post-operation cleanup to reset token allowances after bridge transfers, reducing leftover permissions.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

Modified ERC20 approval flow in ApproveAndBridge.sol: cache bridgeApprovalTarget(), check current allowance, attempt direct low-level approve and fall back to forceApprove on failure, and add a best-effort post-bridge step to zero any remaining allowance for non-native tokens.

Changes

Cohort / File(s)	Summary
ERC20 Approval & Cleanup Logic src/mixin/ApproveAndBridge.sol	Cache bridgeApprovalTarget() in target; check current allowance before approving; attempt direct low-level approve(target, balance) and on failure call token.forceApprove(target, balance); after bridging, best-effort silent low-level call to reset non-zero allowance to 0 for non-native tokens.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related issues

• [Security] Denial of Service (DoS) in approveAndBridge for Zero-Intolerant Tokens #5: Matches the DoS mitigation described—adds allowance checks, tries direct approve before forceApprove, and performs silent post-bridge zeroing.

Poem

🐇 I hopped to the bridge with a careful paw,
Tried a gentle call, then a stronger law,
If leftover crumbs lingered after the feat,
I wiped them away — tidy and neat. ✨

🚥 Pre-merge checks | ✅ 3

✅ 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 clearly and specifically identifies the main change: fixing a DoS vulnerability for zero-intolerant tokens in the approveAndBridge function. It is concise, directly related to the changeset, and references the issue being fixed.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@src/mixin/ApproveAndBridge.sol`:
- Around line 41-49: The current low-level approve call only checks the call
success flag and can miss tokens that return false; change the logic to inspect
the returned data: after calling
address(token).call(abi.encodeWithSelector(IERC20.approve.selector, target,
balance)) capture (bool success, bytes memory returndata) and consider the
approve successful only if success is true AND (returndata.length == 0 ||
abi.decode(returndata, (bool)) == true); otherwise call
token.forceApprove(target, balance). Update the code paths referencing
IERC20.approve.selector, the local variables success/returndata, and
token.forceApprove to implement this check.