code-423n4 / 2022-06-infinity-findings

4 stars 0 forks source link

_getCurrentPrice() in InfinityExchange.sol overlaps with InfinityOrderBookComplication.sol #271

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/core/InfinityExchange.sol#L1153-L1165

Vulnerability details

Impact

InfinityExchange.sol and InfinityOrderBookComplication.sol has the same function _getCurrentPrice().

Documentation defines “Modular architecture”: https://github.com/code-423n4/2022-06-infinity#modular-architecture InfinityExchange.sol - main contract that stores state and has user approvals for spending assets InfinityOrderBookComplication.sol - our first complication that helps execute the order types listed above

InfinityExchange.sol will stay the same contract but Complications will change so _getCurrentPrice() should be removed from InfinityExchange.sol and only InfinityOrderBookComplication.sol should be used. If the duplicate will stay then there can be price mismatch between InfinityExchange.sol and future Complication contract.

Recommended Mitigation Steps

_getCurrentPrice() should be removed from InfinityExchange.sol and only InfinityOrderBookComplication._getCurrentPrice() should be used

InfinityExchange._getCurrentPrice() https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/core/InfinityExchange.sol#L1153-L1165 InfinityOrderBookComplication._getCurrentPrice() https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/core/InfinityOrderBookComplication.sol#L330-L342

HardlyDifficult commented 2 years ago

Architecture feedback is non-critical feedback. Lowering risk and merging with the warden's QA report #273