Tadle
Tadle
DeFi
30,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

Permanent loss of founds for smart contract wallet users if they create orders with WETH

0xlrivo

Summary

Permanent loss of founds for smart contract wallet users if they create orders with WETH

Vulnerability Details

In PreMarket:createOffer() users can specify the token they whish to use which will be deposited in TokenManager for them to later withdraw it.

tokenManager.tillIn{value: msg.value}(
_msgSender(),
params.tokenAddress,
transferAmount,
false
);

Then, when the offer will be closed/settled, TokenManager:addTokenBalance() will be called to register funds that the user can withdraw only with the original token used to create the offer:

function closeOffer(address _stock, address _offer) external {
// ...
tokenManager.addTokenBalance(
TokenBalanceType.MakerRefund,
_msgSender(),
makerInfo.tokenAddress, // <@ original token used to create the offer
refundAmount
);
​
// ...
}

When the user finally decides to withdraw he will call TokenManager:withdraw() and receive back his ERC20s or ETH if he created the offer with WETH.

If the user can claim WETH we will enter in the following if case which contains the critical issue:

if (_tokenAddress == wrappedNativeToken) {
​
// pull the necessary WETH from CapitalPool
_transfer(
wrappedNativeToken,
capitalPoolAddr,
address(this),
claimAbleAmount,
capitalPoolAddr
);
​
// unwrap WETH into ETH
IWrappedNativeToken(wrappedNativeToken).withdraw(claimAbleAmount);
​
// @audit-issue will fail for AA/smart contract wallet users
payable(msg.sender).transfer(claimAbleAmount);
}

The critical issue is identified in the use of the primitive transfer() which enforces a 2300 gas unit limit on the receiver and so will revert if his receive() function uses more than 2300 gas unit to execute.

For instance, the receive function of the Safe Multisig Wallet will require more then 6k gas to reach the implementation contract and emit an event.

Impact

Account Abstraction / smart contract wallet users won't be able to withdraw if they open their position in WETH since their receive() function will probably consume more than 2300 gas.

Even for owners there's no way to recover those funds, for example by changing the withdrawn token, since TokenManager:addTokenBalance is only callable by PreMarkets and DeliveryPlace.

if (_msgSender != preMarketsAddr && _msgSender != deliveryPlaceAddr) {
revert CallerIsNotRelatedContracts(_msgSender);
}

POC

Add the following test to PreMarket.t.sol:

contract MockWallet {
uint256 bal;
​
event Log4(uint256 a, uint256 b, uint256 c, uint256 d);
​
// just to consume more than 2300 gas
receive() external payable {
bal += msg.value; // SSTORE -> 100
emit Log4(bal, msg.value, 0, 0); // LOG 4 -> 1875 gas
emit Log4(msg.value, bal, 0, msg.value); // LOG 4 -> 1875 gas
}
​
}
​
function test_permanent_dos_smart_contract_wallets() public {
MockWallet wallet = new MockWallet();
​
vm.label(address(wallet), "seller");
vm.label(user1, "buyer");
​
deal(address(mockPointToken), address(wallet), 5 ether);
deal(address(wallet), 1 ether);
deal(user1, 1 ether);
vm.startPrank(address(wallet));
// vende 1000 punti a 0.01 ETH
preMarktes.createOffer{value: 0.012 * 1e18}(
CreateOfferParams(
marketPlace,
address(weth9),
1000,
0.01 * 1e18,
10000,
300,
OfferType.Ask,
OfferSettleType.Protected
)
);
​
vm.stopPrank();
​
// user1 buys 500 points
vm.startPrank(user1);
address offerAddr = GenerateAddress.generateOfferAddress(0);
preMarktes.createTaker{value: 0.005175 * 1e18}(offerAddr, 500);
​
systemConfig.updateMarket(
"Backpack",
address(mockPointToken),
0.01 * 1e18,
block.timestamp - 1,
3600
);
vm.stopPrank();
​
vm.startPrank(address(wallet));
mockPointToken.approve(address(tokenManager), type(uint256).max);
deliveryPlace.settleAskMaker(offerAddr, 500);
​
vm.expectRevert(); // <@ Out Of Gas
tokenManager.withdraw(address(weth9), TokenBalanceType.SalesRevenue);
vm.stopPrank();
}

Tools Used

  • Manual review

  • Foundry

Recommendations

Use call() instead of transfer() which doesn't enforce the 2300 gas limit.

Updates

Lead Judging Commences

0xnevi Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Known issue
Assigned finding tags:

[invalid] finding-TokenManager-withdraw-transfer-2300-gas

Invalid, known issues [Medium-2](https://github.com/Cyfrin/2024-08-tadle/issues/1)

Support

FAQs

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