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

the LLMOracleCoordinator.request function ignores the return value of the transferFrom function called on the feeToken contract.

nitinaimshigh

Summary : In Solidity, many functions return values that indicate the success or failure of the operation. For example, the transferFrom 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.

function request(
bytes32 protocol,
bytes memory input,
bytes memory models,
LLMOracleTaskParameters calldata parameters
) public onlyValidParameters(parameters) returns (uint256) {
(uint256 totalfee, uint256 generatorFee, uint256 validatorFee) = getFee(parameters);
​
// check allowance requirements
uint256 allowance = feeToken.allowance(msg.sender, address(this));
if (allowance < totalfee) {
revert InsufficientFees(allowance, totalfee);
}
​
// ensure there is enough balance
uint256 balance = feeToken.balanceOf(msg.sender);
if (balance < totalfee) {
revert InsufficientFees(balance, totalfee);
}
​
// transfer tokens
feeToken.transferFrom(msg.sender, address(this), totalfee);
​
// increment the task id for later tasks & emit task request event
uint256 taskId = nextTaskId;
unchecked {
++nextTaskId;
////more code////

Vulnerability Details : In the request function, the transferFrom function is called to transfer tokens from the sender to the contract. If the transferFrom function fails for some reason (e.g., the sender does not have enough tokens, the contract is not authorized to transfer tokens from the sender, etc.), the request function will not be aware of the failure and will continue executing as if the transfer was successful.

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

  • The contract's balance is not updated correctly

  • The sender'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 transferFrom function and handle any potential errors.

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

Alternatively, you can also use a more explicit error handling approach( second method). This approach allows you to handle the transfer failure in a more explicit way, but it's generally recommended to use the require statement for simplicity and readability.

function request(bytes32 \_taskId, bytes \_input, bytes \_params, LLMOracleTaskParameters \_taskParams) public {
// ...
require(feeToken.transferFrom(msg.sender, address(this), totalfee), "Transfer failed");
// ...
}
//////Another Method/////
​
function request(bytes32 _taskId, bytes _input, bytes _params, LLMOracleTaskParameters _taskParams) public {
// ...
bool success = feeToken.transferFrom(msg.sender, address(this), totalfee);
if (!success) {
// Handle transfer failure
revert("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.

Give us feedback!