BondCallback Re-Entrancy vulnerability

[code423n4]

Lines of code

https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/BondCallback.sol#L127 https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/BondCallback.sol#L143 https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/BondCallback.sol#L144

Vulnerability details

Impact

when withdraw reserves from TRSRY to msg.sender,it may go to other external uncontrollable contract logic if reserve token contract transferFrom function call to other contract ,it will cause other market use this callback asset loss or this contract amountsPerMarket can't match real amount.

Proof of Concept

priorBalances[quoteToken] = quoteToken.balanceOf(address(this));
priorBalances[payoutToken] = payoutToken.balanceOf(address(this));
_amountsPerMarket[id_][0] += inputAmount_;
_amountsPerMarket[id_][1] += outputAmount_;

when withdraw reserves from TRSRY to msg.sender,it may go to other external uncontrollable contract logic if reserve token contract transferFrom function call to other contract （like EIP1271 to valid signature).it will cause amountsPerMarket has not update，but attacker utilize it logic to call other market or some contract use unupdated amountsPerMarket data(re-entrant use amountsForMarket function),it will make some market asset loss.Attacker also can use it logic to transfer some payout token(if this token don't have nonReentrant) letpriorBalances can't match real priorBalances , and let amountsPerMarket can't match real amount.

Recommended Mitigation Steps

modify priorBalances and _amountsPerMarket before line 127 ：

TRSRY.withdrawReserves(msg.sender, payoutToken, outputAmount_);

it can avoid Re-Entrancy.

[bahurum]

I'd say this is invalid. Didn't fully get PoC, but callback() is nonReentrant, so I would say there is no re-entrancy issue.

[Oighty]

Agree with @bahurum .

[0xean]

closing as invalid.