15,000 USDC
View results
Submission Details
Severity: gas

(**Note)

Summary

Revisiting before submission: this is now a known issue. While I am super stoked and so thankful to be apart of this audit, the only disheartening thing I would mention, it's been hard to track what are known issues vs more known issues when it tends to only be on the announcements page instead of the repo. For example Known Issues are on this Stablecoin repo page, but there is no known issues on the Escrow or Beedle repo pages.

I would check the repo, see a known issue, then double check the announcements discord tab and see more issues. So I was somewhat confused, and I image others may be if they didn't know to check the announcements page. But please disregard this submission.

Per the discord discussion, other tokens may be used other than wETH and wBTC. Therefore, it should be noted not all tokens implement transferFrom. Some tokens (like USDT) don't correctly implement the EIP20 standard and their transfer/transferFrom function return void instead of a successful boolean. Calling these functions with the correct EIP20 function signatures will always revert.

Vulnerability Details

157: bool success = IERC20(tokenCollateralAddress).transferFrom(msg.sender, address(this), amountCollateral);
274: bool success = i_dsc.transferFrom(dscFrom, address(this), amountDscToBurn);
287: bool success = IERC20(tokenCollateralAddress).transfer(to, amountCollateral);

Impact

If the protocol intends to accept other tokens, tokens that don't correctly implement the latest EIP20 spec, like USDT, they will be unusable in the protocol as they revert the transaction because of the missing return value.

Tools Used

Manual Review

Recommendations

I recommend using OpenZeppelin’s SafeERC20 versions with the safeTransfer and safeTransferFrom functions that handle the return value check as well as non-standard-compliant tokens.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.