Egis-Security / CTF_Challenge

Repository containing CTF challenges from nmirchev8, deth and bOgO.
14 stars 8 forks source link

b0g0_ctf - Unauthorized Withdrawal via NFT Transfer #35

Open mo-hak opened 2 months ago

mo-hak commented 2 months ago

๐—ฆ๐—ฒ๐˜ƒ๐—ฒ๐—ฟ๐—ถ๐˜๐˜†:

Medium

Description of the Bug:

mapping(address => uint256) public deposits; When a user deposits ETH and receives an NFT, the ๐๐ž๐ฉ๐จ๐ฌ๐ข๐ญ๐ฌ ๐ฆ๐š๐ฉ๐ฉ๐ข๐ง๐  is updated to reflect the deposited amount mapped to the initial depositor. However, if the NFT is transferred to another address via ๐ญ๐ซ๐š๐ง๐ฌ๐Ÿ๐ž๐ซ ๐Ÿ๐ฎ๐ง๐œ๐ญ๐ข๐จ๐ง from erc721 (as it is not overridden), ๐ญ๐ก๐ž ๐๐ž๐ฉ๐จ๐ฌ๐ข๐ญ๐ฌ ๐ฆ๐š๐ฉ๐ฉ๐ข๐ง๐  ๐ข๐ฌ ๐ง๐จ๐ญ ๐ฎ๐ฉ๐๐š๐ญ๐ž๐ ๐ญ๐จ ๐ซ๐ž๐Ÿ๐ฅ๐ž๐œ๐ญ ๐ญ๐ก๐ข๐ฌ ๐ญ๐ซ๐š๐ง๐ฌ๐Ÿ๐ž๐ซ ๐ญ๐จ ๐ญ๐ก๐ž ๐ง๐ž๐ฐ ๐จ๐ฐ๐ง๐ž๐ซ. This allows the new owner of the NFT to withdraw ETH without having deposited any, leading to unauthorized withdrawals.

Impact:

Underflow Errors: Subtracting the deposit amount from an address with zero balance could cause underflow errors, leading to unexpected behavior or contract failure. And the original owner cant also withdraw as it is not the current owner. This would lead to ๐๐ž๐ง๐ข๐š๐ฅ ๐จ๐Ÿ ๐ฌ๐ž๐ซ๐ฏ๐ข๐œ๐ž

Solution:

To prevent the vulnerability, the contract should be modified to ensure that only the original depositor can withdraw their ETH. This can be achieved by tracking the original depositor for each token ID and ensuring that only this address can call the withdraw function.

BogoCvetkov commented 2 months ago

Valid! Already submitted by another auditor -> https://github.com/Egis-Security/CTF_Challenge/issues/22