The Standard
The Standard
DeFiHardhat
20,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

`_afterTokenTransfer` can make `transfer` fail for a user

00xSEV

Summary

SmartVaultManagerV5::_afterTokenTransfer call smartVaultIndex.transferTokenId(_from, _to, _tokenId), that will make transfer exceed block gas limit if _from has too many tokens.

Vulnerability Details

smartVaultIndex.transferTokenId gas cost grow lineary with each token in _from. When a token is transferred _afterTokenTransfer is called.
An attacker can mint ~ 30_000_000 / 5000 = 6000 tokens, transfer to a victim to lock the transfer function. Or a user may create many vaults themself.

Impact

Locked transfer functionality

Proof of Concept

You can put it in tests folder, run npx hardhat test test/fileName.js and check the console output

// @ts-check
const { ethers } = require("hardhat");
​
​
let SmartVaultIndex, deployer, victim, receiver;
​
describe('SmartVault', async () => {
beforeEach(async () => {
([deployer, victim, receiver] = await ethers.getSigners());
SmartVaultIndex = await (await ethers.getContractFactory('SmartVaultIndex')).deploy();
await SmartVaultIndex.setVaultManager(deployer.address);
});
​
describe('count gas', async () => {
it('1', async () => {
const gasUsed = [];
​
for (let i = 1; i <= 30; i++){
const {AddressZero} = ethers.constants;
const victimAddress = victim.address;
// Just to send back and forth to count gas
const receiverAddress = receiver.address;
const tokenId = i;
// mint
await SmartVaultIndex.transferTokenId(AddressZero, victimAddress, tokenId);
// transfer to get the gas cost
const transferTx = await SmartVaultIndex.transferTokenId(victimAddress, receiverAddress, tokenId);
// transfer back so the victim has all the tokens
await SmartVaultIndex.transferTokenId(receiverAddress, victimAddress, tokenId);
​
// Note: count initial transfer, victim->receiver
// Each loop victim will have more and more tokens
const receipt = await transferTx.wait();
gasUsed.push(receipt.gasUsed);
}
​
​
// ignore the first call because it SSTORE some slots from 0 to value, which is more expensive
// delta ~4736 gas on my environment
// so only ~6000 txs to overflow block gas limit (30mln)
console.log(`getDeltas (diff between prev and current tx in gas)`, getDeltas(gasUsed));
console.log(`gasUsed`, gasUsed.map(bn => bn.toNumber()));
});
});
});
​
function getDeltas(gasUsed) {
const deltas = [];
​
for (let i = 1; i < gasUsed.length; i++) {
const delta = gasUsed[i].sub(gasUsed[i - 1]);
deltas.push(delta.toNumber());
}
​
return deltas;
}

Tools Used

Manual review

Recommended Mitigation Steps

Consider limiting the number of tokens a user can have.
Consider using a different smartVaultIndex implementation that does not increase gas consumption lineary

Updates

Lead Judging Commences

hrishibhat Lead Judge almost 2 years ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

informational/invalid

Support

FAQs

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

Give us feedback!