Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rebasing Token's increased token amount will be locked up forever #223

Closed
code423n4 opened this issue Nov 8, 2022 · 2 comments
Closed
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-47 satisfactory satisfies C4 submission criteria; eligible for awards

Comments

@code423n4
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2022-11-size/blob/706a77e585d0852eae6ba0dca73dc73eb37f8fb6/src/SizeSealed.sol#L55-L56
https://github.com/code-423n4/2022-11-size/blob/706a77e585d0852eae6ba0dca73dc73eb37f8fb6/src/interfaces/ISizeSealed.sol#L77-L78

Vulnerability details

Impact

Tokens such as Aave aTokens are rebasing token where its token amount will increase over time.
Since SizeSealed contract reference deposited token amount from user input, any token amount that increased over time
will not be referenced which means increased token amount will be locked in the contract forever.
Also since SizeSealed contract has no limitation for what token can be used as baseToken and quoteToken, this issue
falls for both baseToken and quoteToken.

aTokens reference:
https://edge.app/blog/company-news/interest-bearing-tokens-in-edge-atokens-ctokens/

Proof of Concept

Any token can be specified for both baseToken and quoteToken at createAuction function.
https://github.com/code-423n4/2022-11-size/blob/706a77e585d0852eae6ba0dca73dc73eb37f8fb6/src/SizeSealed.sol#L55-L56
https://github.com/code-423n4/2022-11-size/blob/706a77e585d0852eae6ba0dca73dc73eb37f8fb6/src/interfaces/ISizeSealed.sol#L77-L78

  1. baseToken

createAuction function:
Auction seller sending its specified basetoken amount to SizeSealed contract.
https://github.com/code-423n4/2022-11-size/blob/706a77e585d0852eae6ba0dca73dc73eb37f8fb6/src/SizeSealed.sol#L56
https://github.com/code-423n4/2022-11-size/blob/706a77e585d0852eae6ba0dca73dc73eb37f8fb6/src/interfaces/ISizeSealed.sol#L80
https://github.com/code-423n4/2022-11-size/blob/706a77e585d0852eae6ba0dca73dc73eb37f8fb6/src/SizeSealed.sol#L98-L100

 98:        SafeTransferLib.safeTransferFrom(
 99:            ERC20(auctionParams.baseToken), msg.sender, address(this), auctionParams.totalBaseAmount
100:        );

finalize function:
When token are sent back to seller, the token amount is referenced from the totalBaseAmount which was specified by the
seller when executing createAuction function, meaning that any increased token amount is ignored.
https://github.com/code-423n4/2022-11-size/blob/706a77e585d0852eae6ba0dca73dc73eb37f8fb6/src/SizeSealed.sol#L217-322

217:    function finalize(uint256 auctionId, uint256[] memory bidIndices, uint128 clearingBase, uint128 clearingQuote)
...
221:        Auction storage a = idToAuction[auctionId];
...
233:        data.totalBaseAmount = a.params.totalBaseAmount;
...
318:        if (data.totalBaseAmount != data.filledBase) {
319:            uint128 unsoldBase = data.totalBaseAmount - data.filledBase;
320:            a.params.totalBaseAmount = data.filledBase;
321:            SafeTransferLib.safeTransfer(ERC20(a.params.baseToken), a.data.seller, unsoldBase);
322:        }

cancelAuction function:
Same as finalize function explained above.
https://github.com/code-423n4/2022-11-size/blob/706a77e585d0852eae6ba0dca73dc73eb37f8fb6/src/SizeSealed.sol#L391-L392
https://github.com/code-423n4/2022-11-size/blob/706a77e585d0852eae6ba0dca73dc73eb37f8fb6/src/SizeSealed.sol#L409

391:    function cancelAuction(uint256 auctionId) external {
392:        Auction storage a = idToAuction[auctionId];
...
409:        SafeTransferLib.safeTransfer(ERC20(a.params.baseToken), msg.sender, a.params.totalBaseAmount);
  1. quoteToken

bid function:
Bidder sending its specified quoteAmount amount to SizeSealed contract.
https://github.com/code-423n4/2022-11-size/blob/706a77e585d0852eae6ba0dca73dc73eb37f8fb6/src/SizeSealed.sol#L124
https://github.com/code-423n4/2022-11-size/blob/706a77e585d0852eae6ba0dca73dc73eb37f8fb6/src/SizeSealed.sol#L163

122:    function bid(
124:        uint128 quoteAmount,
...
163:        SafeTransferLib.safeTransferFrom(ERC20(a.params.quoteToken), msg.sender, address(this), quoteAmount);

refund function:
When token are sent back to bidder, the token amount is referenced from the quoteAmount which was specified by the
bidder when executing bid function, meaning that any increased token amount is ignored.
https://github.com/code-423n4/2022-11-size/blob/706a77e585d0852eae6ba0dca73dc73eb37f8fb6/src/SizeSealed.sol#L336-L338
https://github.com/code-423n4/2022-11-size/blob/706a77e585d0852eae6ba0dca73dc73eb37f8fb6/src/SizeSealed.sol#L351

336:    function refund(uint256 auctionId, uint256 bidIndex) external atState(idToAuction[auctionId], States.Finalized) {
337:        Auction storage a = idToAuction[auctionId];
338:        EncryptedBid storage b = a.bids[bidIndex];
...
351:        SafeTransferLib.safeTransfer(ERC20(a.params.quoteToken), msg.sender, b.quoteAmount);

withdraw function:
Same as refund function explained above.
https://github.com/code-423n4/2022-11-size/blob/706a77e585d0852eae6ba0dca73dc73eb37f8fb6/src/SizeSealed.sol#L376-L381

376:        if (b.quoteAmount != 0) {
377:            uint256 quoteBought = FixedPointMathLib.mulDivDown(baseAmount, a.data.lowestQuote, a.data.lowestBase);
378:            uint256 refundedQuote = b.quoteAmount - quoteBought;
379:            b.quoteAmount = 0;
380:
381:            SafeTransferLib.safeTransfer(ERC20(a.params.quoteToken), msg.sender, refundedQuote);

cancelBid function:
Same as refund function explained above.
https://github.com/code-423n4/2022-11-size/blob/706a77e585d0852eae6ba0dca73dc73eb37f8fb6/src/SizeSealed.sol#L439

439:        SafeTransferLib.safeTransfer(ERC20(a.params.quoteToken), msg.sender, b.quoteAmount);

Tools Used

Manual Analysis

Recommended Mitigation Steps

Add whitelist logic to limit what kind of token is allowed to set as baseToken and quoteToken or
track total amount currently deposited and allow seller/bidder to withdraw token amount that increased over time.

@code423n4 code423n4 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Nov 8, 2022
code423n4 added a commit that referenced this issue Nov 8, 2022
@c4-judge
Copy link
Contributor

c4-judge commented Nov 9, 2022

0xean marked the issue as duplicate

@c4-judge
Copy link
Contributor

c4-judge commented Dec 6, 2022

0xean marked the issue as satisfactory

@c4-judge c4-judge added the satisfactory satisfies C4 submission criteria; eligible for awards label Dec 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-47 satisfactory satisfies C4 submission criteria; eligible for awards
Projects
None yet
Development

No branches or pull requests

3 participants