Dria
Swan
NFTHardhat
21,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

BuyerAgent.withdraw function ignores the return value of the transfer function called on the swan.token() contract.

nitinaimshigh

Summary : In Solidity, many functions return values that indicate the success or failure of the operation. For example, the transfer function typically returns a boolean value indicating whether the transfer was successful or not.

Ignoring these return values can lead to unexpected behavior and potential security vulnerabilities. If a function fails, but the return value is ignored, the contract may continue executing as if the operation was successful.

Vulnerability Details : In the withdraw function, the transfer function is called to transfer tokens from the contract to the owner. If the transfer function fails for some reason (e.g., the token contract is paused, the recipient is not a valid address, etc.), the withdraw function will not be aware of the failure and will continue executing as if the transfer was successful.

This can lead to inconsistent state, such as:

  • The contract's balance is not updated correctly

  • The owner's balance is not updated correctly

  • The contract's state is not updated correctly

function withdraw(uint96 _amount) public onlyAuthorized {
(, Phase phase,) = getRoundPhase();
​
// if we are not in Withdraw phase, we must leave
// at least minFundAmount in the contract
if (phase != Phase.Withdraw) {
// instead of checking `treasury - _amount < minFoundAmount`
// we check this way to prevent underflows
if (treasury() < minFundAmount() + _amount) {
revert MinFundSubceeded(_amount);
}
}
​
// transfer the tokens to the owner of Buyer
swan.token().transfer(owner(), _amount);
}

Impact : This can lead to inconsistent state, such as:

  • The contract's balance is not updated correctly

  • The owner's balance is not updated correctly

  • The contract's state is not updated correctly

Tools Used : Slither, VS code

Recommendations : To fix this issue, you should check the return value of the transfer function and handle any potential errors.

By adding the require statement, you ensure that the function will revert if the transfer function fails, preventing any potential security vulnerabilities.

Alternatively, you can also use a more explicit error handling approach, as mentioned in first method.

function withdraw(uint96 \_amount) public {
// ...
bool success = swan.token().transfer(owner(), _amount);
if (!success) {
// Handle transfer failure
revert("Transfer failed");
}
// ...
}
​
//// Another approach //////
function withdraw(uint96 _amount) public {
// ...
require(swan.token().transfer(owner(), _amount), "Transfer failed");
// ...
}
​
Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Known issue
Assigned finding tags:

[KNOWN] - Low-35 Unsafe use of transfer()/transferFrom() with IERC20

Support

FAQs

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