code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-05-rubicon/blob/main/contracts/rubiconPools/BathToken.sol#L16

Vulnerability details

Impact

No implementation of decrease/increase allowance are a risk to fronton attacks

Proof of Concept

No decrease/increase allowance are a really bug risk for users that are now vulnerable to fronton attacks when using approve For example let's say Bob gives Alice and allowance of x using approve. Now let's say Alice still has not used those allowed tokens, and bob wants to changed the allowance to y. Alice can fronton Bob's transaction and use those x allowed tokens, now the allowance of bob to Alice decreases to 0. Now bob's transaction performed and the allowance changed again to y. Now Alice can use those y tokens too.

Total: Alice used x+y of bob's tokens when he never meant for that to happen.

Tools Used

Recommended Mitigation Steps

Implement increase and decrease allowance functions and in that way the attack won't be possible.

bghughes commented 2 years ago

Incorrect logic IMO and not relevant here. Global comment about ERC-20 approval patterns that seems flawed because it applies anywhere in EVM

KenzoAgada commented 2 years ago

Warden just describes the general approval issue. Didn't link to specific LOC, didn't mention how it is relevant to the project. I think this is negative value and marking it as "high" even more so.

pauliax commented 2 years ago

I believe we agreed to treat such issues with a low severity unless the protocol was specifically designed to combat it: https://github.com/byterocket/c4-common-issues/blob/main/2-Low-Risk.md#l001---unsafe-erc20-operations

HickupHH3 commented 2 years ago

I'm tempted to invalidate it to discourage low-quality generic submissions. Ideally, wardens should submit issues that provide meaningful context and value to the sponsor.

Nevertheless, as what Pauliax pointed out, I'll downgrade it to QA.

Edit: Issue is merely a best practice. In reality, most contract interactions use the safeApprove() / safeIncreaseAllowance() / safeDecreaseAllowance() functions of the SafeERC20 library, which calls the token's approve() function.

Given its limited usage, severity at QA stands.