code-423n4 / 2021-12-defiprotocol-findings

0 stars 0 forks source link

Broken unit tests due to incorrect values #88

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Handle

bw

Vulnerability details

Impact

The unit tests were not passing, this means that certain functionality of the code could be untested and any future developments are more likely to introduce bugs.

The values used in the Auction.test.js#L160 were incorrect.

Proof of Concept

$ npx hardhat run test

Basket
    1) should allow settling an auction by the auction bonder
    ✓ should allow minting if user has all needed balances and approvals (127ms)

  4 passing (3s)
  1 failing

  1) Auction
       should allow settling an auction by the auction bonder:
     Uncaught Error: Transaction reverted without a reason string
      at Auction.settleAuction (contracts/Auction.sol:107)

Tools Used

N/A

Recommended Mitigation Steps

Update the unit test to use the correct values.

diff --git a/contracts/test/Auction.test.js b/contracts/test/Auction.test.js
index 1b2e4fd..923e64a 100644
--- a/contracts/test/Auction.test.js
+++ b/contracts/test/Auction.test.js
@@ -157,6 +157,6 @@ describe("Auction", function () {
         await COMP.approve(auction.address, `100000000000000000000`);
         await AAVE.approve(auction.address, `100000000000000000000`);

-        await expect(auction.settleAuction([], [COMP.address], ["2999400000000000000"], [UNI.address, AAVE.address], ["200720000000000000", "200120000000000000"])).to.be.ok;
+        await expect(auction.settleAuction([], [COMP.address], ["2999600000000000000"], [UNI.address, AAVE.address], ["200480000000000000", "200080000000000000"])).to.be.ok;
       });
 });
$ npx hardhat run test

Auction
    ✓ should allow bonding by one user when there is an auction ongoing (89ms)
    ✓ should allow burning the bond if an auction hasn't been settled in 24 hours (70ms)
    ✓ should allow settling an auction by the auction bonder (106ms)

  Basket
    ✓ should allow minting if user has all needed balances and approvals (126ms)
    ✓ should allow burning if user has basket tokens (128ms)

  Factory
    ✓ Should add a valid proposal
    ✓ Should Revert on Duplicate Tokens
    ✓ Should Revert on < minLicenseFee
    ✓ Should Revert on zero values
    ✓ Should Revert on mismatched Arrays
    ✓ Should create a valid basket and mint 1 Basket token to sender (250ms)
    ✓ Should fail basket creation if creator doesn't have enough tokens or not approved (285ms)

  12 passing (5s)
frank-beard commented 2 years ago

thank you for the feedback.

0xleastwood commented 2 years ago

No security risk, so marking as non-critical.