code423n4 commented 2 years ago

Handle

0x1f8b

Vulnerability details

Impact

Wrong logic around safe transfers.

Proof of Concept

The methods safeTransfer and safeTransferFrom from BoringERC20 library has the following condition:

require(success && (data.length == 0 || abi.decode(data, (bool))), "BoringERC20: Transfer failed");

the ERC20 standard define that always these two methods will return a boolean value, and it's possible to bypass this call with an empty return, if this call returns empty value, it will be with bad intentions because otherwise it will implement the ERC20 interface. It's mandatory to check that the returns it's a valid boolean value.

Tools Used

Manual review.

Recommended Mitigation Steps

Check that always return a boolean

kingyetifinance commented 2 years ago

@LilYeti: Duplicate #1 and severity medium

kingyetifinance commented 2 years ago

These functions are unused in the new codebase

alcueca commented 2 years ago

Duplicate of #94