code-423n4 / 2022-06-nibbl-findings

1 stars 0 forks source link

QA Report #293

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

1. unused import statments

nibblVaultFactory.sol is already importing everything from NibblVault.sol which has Ierc20.sol ,Ierc1155.sol,IERC721.sol already imported from it . Remove Ierc20.sol and Ierc1155.sol ,IERC721.sol from nibblVaultFactory.sol. instances include: nibblVaultFactory.sol :4,5,6

2. there are no emits of events after updateing importent variables and the old variable

nibblVaultFactory.sol :102,109,125,143,160,168

no check on address zero

nibblVaultFactory.sol :100,123,140,158

not use erc20upgradeable it can brake

if it gets upgraded and cause weird logic nibblvault:1

typos

instead of seconday use:secondary https://github.com/NibblNFT/nibbl-smartcontracts/blob/49bf364d9e81a554cfdf47ae5cfc3daf52a54ad6/contracts/NibblVault.sol#L263 instead of continous use: continuos https://github.com/NibblNFT/nibbl-smartcontracts/blob/49bf364d9e81a554cfdf47ae5cfc3daf52a54ad6/contracts/NibblVault.sol#L282 instead of : recieve use: receive https://github.com/NibblNFT/nibbl-smartcontracts/blob/49bf364d9e81a554cfdf47ae5cfc3daf52a54ad6/contracts/NibblVault.sol#L361

Attacker can possibly mint tokens he dosnt pay for

Discalimer i dint know if this was an exploit so its in qa _minAmtOut dosent have to fail if you supply zero becaue anything in purchaseReturn is greater or equal to it. attacker can take advangate to this possibly steal tokens or brake logic and maybe even mint tokens for himself if attacker dosnt pay the fees and if purchaseReturn is more than zero lets say 1 wei then no check for such a small amount that he is going to work and possible get through alot of bypass of your contract.

  1. Attacker calls buy() function and _minAmtOut=0 and _to=AttackerADDRERESS
  2. since there is no check for anything that small of amount attacker dosnt pay fees.
  3. mints more then 1 wei of tokens then supply and contract execpted and since there is no _minAmtOut check
  4. Attacker mints small amont of tokens at time same thing with the sell function

    mitigation

    require (_minAmtOut>0);

HardlyDifficult commented 2 years ago

Merging with https://github.com/code-423n4/2022-06-nibbl-findings/issues/254

HardlyDifficult commented 2 years ago

Good suggestions, very succinct