code-423n4 / 2021-06-tracer-findings

1 stars 0 forks source link

Wrong token approval #99

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

cmichel

Vulnerability details

The pool holdings of Insurance (publicCollateralAmount and bufferCollateralAmount) is in WAD (18 decimals) but it's used as a raw token value in drainPool

// amount is a mix of pool holdings, i.e., 18 decimals
// this requires amount to be in RAW! if tracerMarginToken has > 18 decimals, it'll break, < 18 decimals will approve too much
tracerMarginToken.approve(address(tracer), amount);
// this requires amount to be in WAD which is correct
tracer.deposit(amount);

Impact

If tracerMarginToken has less than 18 decimals, the approval approves orders of magnitude more tokens than required for the deposit call that follows. If tracerMarginToken has more than 18 decimals, the deposit that follows would fail as fewer tokens were approved, but the protocol seems to disallow tokens in general with more than 18 decimals.

Recommended Mitigation Steps

Convert the amount to a "raw token value" and approve this one instead.

raymogg commented 3 years ago

The issue is correct in pointing out that the wrong approve amount is used, however disagree with the severity.

It is common practice to approve the maximum amount of tokens for a contract to spend already. This bug simply allows more tokens to be approved (to a trusted contract in the system), than was intended. This is only exploitable if paired with another bug in the Tracer contracts. As is, no users would be affected.

cemozerr commented 3 years ago

Marking this as low-risk as it would only pose a security threat coupled with another bug.