code-423n4 / 2022-07-swivel-findings

0 stars 1 forks source link

removeNotional() does not judge whether `a` is 0 #81

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-07-swivel/blob/main/Creator/VaultTracker.sol#L82

Vulnerability details

Impact

removeNotional() does not judge whether a is 0

Proof of Concept

https://github.com/code-423n4/2022-07-swivel/blob/main/Creator/VaultTracker.sol#L82 https://github.com/code-423n4/2022-07-swivel/blob/main/VaultTracker/VaultTracker.sol#L82

File: Creator/VaultTracker.sol

L82: 
function removeNotional(address o, uint256 a) external authorized(admin) returns (bool) {
  Vault memory vlt = vaults[o];

  if (a > vlt.notional) { revert Exception(31, a, vlt.notional, address(0), address(0)); }

  uint256 exchangeRate = Compounding.exchangeRate(protocol, cTokenAddr);

  // if market has matured, calculate marginal interest between the maturity rate and previous position exchange rate
  // otherwise, calculate marginal exchange rate between current and previous exchange rate.
  uint256 yield;
  if (maturityRate > 0) { // Calculate marginal interest
    yield = ((maturityRate * 1e26) / vlt.exchangeRate) - 1e26;
  } else {
    // calculate marginal interest
    yield = ((exchangeRate * 1e26) / vlt.exchangeRate) - 1e26;
  }

  uint256 interest = (yield * vlt.notional) / 1e26;
  // remove amount from position, Add interest to position, reset cToken exchange rate
  vlt.redeemable += interest;
  vlt.notional -= a;
  vlt.exchangeRate = exchangeRate;

  vaults[o] = vlt;

  return true;
}

There is no judgment on whether a is 0. If a is 0, the notional is unchanged, but redeemable and exchangeRate will be recalculated.

Tools Used

vscode

Recommended Mitigation Steps

JTraversa commented 2 years ago

https://github.com/code-423n4/2022-07-swivel#input-sanitization

bghughes commented 2 years ago

I believe this is QA for the same reasons as said here: https://github.com/code-423n4/2022-07-swivel-findings/issues/80#issuecomment-1200484929

bghughes commented 2 years ago

Grouping this with the warden’s QA report #82