itsabinashb - BaseGladiusReactor.sol::`_fill()`:There is no check for whether the token has code or not

[sherlock-admin2]

itsabinashb

high

BaseGladiusReactor.sol::_fill():There is no check for whether the token has code or not

Summary

In BaseGladiusReactor.sol::_fill() SafeTransferLib is using for ERC20, but the function does not have any check for whether the ERC20 token has any code or not, so an attacker can use a token which does not have any code i.e invalid token & as a result the token will be transfered but will not credited because there is no token exist at all.

Vulnerability Detail

In SafeTransferLib.sol library a note is clearly mentioned:

@dev Note that none of the functions in this library check that a token has code at all! That responsibility is delegated to the caller.

The BaseGladiusReactor.sol contract using SafeTransferLib.sol for ERC20 interactions:

    using SafeTransferLib for ERC20;

In BaseGladiusReactor.sol::_fill() has an external call on token of OutputToken[], this token is need to be received by recipient to satisfy an order. But this _fill() does not have any check for whether the token has any code or not. The external call is called to CurrencyLibrary.sol::transferFill(), in this function the safeTransferFrom() is called to send the token to recipient, the CurrencyLibrary.sol contract also using SafeTransferLib for ERC20. Unfortunately this transferFill() also does not have any check for token code.

Impact

As Rubicon supports trading of any token which adheres to ERC20 standard and one can add a new token so an attacker can use a token which does not have any code i.e invalid token, so when safeTransferFrom() [SafeTransferLib.sol dependant] is called on that token the token transfer will succeed with no error but the token will not be credited to recipient.

Code Snippet

1. https://github.com/sherlock-audit/2024-02-rubicon-finance/blob/main/gladius-contracts-internal/src/lib/CurrencyLibrary.sol#L39-L51
2. https://github.com/sherlock-audit/2024-02-rubicon-finance/blob/main/gladius-contracts-internal/src/reactors/BaseGladiusReactor.sol#L222-L243
3. https://github.com/transmissions11/solmate/blob/c892309933b25c03d32b1b0d674df7ae292ba925/src/utils/SafeTransferLib.sol#L9

Tool used

Manual Review

Recommendation

Use OZ's safeTransferFrom() or put a check for whether the token has a code or not.

[sherlock-admin]

3 comment(s) were left on this issue during the judging contest.

tsvetanovv commented:

Low

PNS commented:

User input validation: User input validation to prevent user mistakes is not considered a valid issue.

0xAadi commented:

Invalid: OOS, custom implementation tokens are OOS