Nibbl contest - SmartSek's results

NFT fractionalization protocol with guaranteed liquidity and price based buyout.

General Information

Platform: Code4rena

Start Date: 21/06/2022

Pot Size: $30,000 USDC

Total HM: 12

Participants: 96

Period: 3 days

Judge: HardlyDifficult

Total Solo HM: 5

Id: 140

League: ETH

Nibbl

Findings Distribution

Researcher Performance

Rank: 11/96

Findings: 3

Award: $1,098.20

🌟 Selected for report: 0

🚀 Solo Findings: 0

Findings Information

🌟 Selected for report: Lambda

Also found by: SmartSek

Labels

bug
duplicate
2 (Med Risk)
sponsor disputed

Awards

1052.705 USDC - $1,052.70

External Links

Lines of code

https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/NibblVault.sol#L300 https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/NibblVault.sol#L362

Vulnerability details

Impact

Impacts are two fold:

  1. admin should not be allowed to stop users from selling/cashing out their assets under any circumstance.

  2. Due to poor timing or malicious admin activity, it could be the case that buy() is put on pause right after initiateBuyout() is called, making it impossible for a bid to be rejected.

Proof of Concept

https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/NibblVault.sol#L153-L156

    modifier whenNotPaused() {
        require(!NibblVaultFactory(factory).paused(), 'NibblVault: Paused');
        _;
    }

https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/NibblVault.sol#L300-L301

    function buy(uint256 _minAmtOut, address _to) external override payable notBoughtOut lock whenNotPaused returns(uint256 _purchaseReturn) {

https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/NibblVault.sol#L362

    function sell(uint256 _amtIn, uint256 _minAmtOut, address payable _to) external override notBoughtOut whenNotPaused returns(uint256 _saleReturn) {

Tools Used

Manual Review

Remove whenNotPaused from sell(). Prevent the pausing of buy() if status = Status.buyout.

#0 - KenzoAgada

2022-06-26T01:10:45Z

Contest readme: "Out of scope: Admin can pause and change certain parameters of the contract."

#1 - fatherGoose1

2022-06-26T10:27:12Z

Understood. Hoping that impact #2 listed above might lead to reconsideration of this design. If many (thousands?) of simultaneous markets are controlled by a single pause authority, its very likely that one or several NFTs will not receive a fair rejection process.

#2 - HardlyDifficult

2022-06-30T00:57:23Z

The report here could have used a better description, particularly if it's reported as a potential High severity. However 2) does touch on the same point as #55 - closing this as a dupe.

QA Report

[L-01] Lack of ownership transfer pattern

Setting the curator to the wrong address will result in permanent loss of functionality in the protocol. It is recommended to apply a two-step transfer.

https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/NibblVault.sol#L485-L488

[L-02] Unnecessary safeTransferFrom() when sending from address(this)

safeTransfer() can be used instead when sending from the current address.

https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/NibblVault.sol#L495-L509

[L-03] Missing zero address check

Contract would need to be redeployed if the feeTo variable is accidently set to address(0).

https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/NibblVaultFactory.sol#L25

[L-04] Unsafe transfer, use safeTransfer()

It is recommended to use safeTransfer() instead of transfer() to mitigate against ERC20 implementations that do not revert on failure.

https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/NibblVault.sol#L517

[N-01] Constants should be all caps

Most constants defined in the contracts are fully capitalized, just this one example is not.

https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/NibblVault.sol#L28

[N-02] CURVE_FEE is described as a variable when it is a constant

https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/NibblVault.sol#L210

#0 - HardlyDifficult

2022-07-03T01:27:12Z

#1 - HardlyDifficult

2022-07-04T19:11:03Z

All good considerations

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter