Low-level call returns true even if the account called is non-existent. Here it calls a market using low level call:
(bool success, ) = makeOrder.market.call(
abi.encodePacked(
ITracerPerpetualSwaps(makeOrder.market).matchOrders.selector,
abi.encode(makeOrder, takeOrder, fillAmount)
)
);
// ignore orders that cannot be executed
if (!success) continue;
If market is not an existing contract, this call will still succeed and will continue the execution. Here is a simple POC that I wrote to demonstrate that: https://gist.github.com/pauliax/54b4bda4f5bfb3d310c169b3427e5b23 Try to execute function lowCall on EOA and see that it succeeds.
raymogg
commented
3 years ago
Handle
pauliax
Vulnerability details
Impact
Low-level call returns true even if the account called is non-existent. Here it calls a market using low level call: (bool success, ) = makeOrder.market.call( abi.encodePacked( ITracerPerpetualSwaps(makeOrder.market).matchOrders.selector, abi.encode(makeOrder, takeOrder, fillAmount) ) ); // ignore orders that cannot be executed if (!success) continue; If market is not an existing contract, this call will still succeed and will continue the execution. Here is a simple POC that I wrote to demonstrate that: https://gist.github.com/pauliax/54b4bda4f5bfb3d310c169b3427e5b23 Try to execute function lowCall on EOA and see that it succeeds.
This issue was described before by ser 0xRajeev: https://github.com/code-423n4/2021-04-basedloans-findings/issues/16 You can read more about it here (read warning sections): https://docs.soliditylang.org/en/v0.8.6/units-and-global-variables.html?highlight=.call#members-of-address-types
Recommended Mitigation Steps
Check against EOA before calling the market or refactor to eliminate low-level call and use the interface.