QA Report

[code423n4]

1. Signature malleability of EVM's ecrecover in verify()

Impact

The permit function of NibblVault calls the Solidity ecrecover function directly to verify the given signatures. However, the ecrecover EVM opcode allows malleable (non-unique) signatures and thus is susceptible to replay attacks.

Although a replay attack seems not possible here since the nonce is increased each time, ensuring the signatures are not malleable is considered a best practice (and so is checking _signer != address(0), where address(0) means an invalid signature).

Proof of Concept

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

SWC-117: Signature Malleability SWC-121: Missing Protection against Signature Replay Attacks

Recommended Mitigation Steps Use the recover function from OpenZeppelin's ECDSA library for signature verification.

2. Wrong revert message in initiateBuyout()

In initiateBuyout() function, it check if block.timestamp >= minBuyoutTime and revert with message minBuyoutTime < now. But the revert message should be

minBuyoutTime > now

Proof of concept

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

3. Typo

• Naming function consistently between initialize in Vault and initialise in Basket

[HardlyDifficult]

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

[HardlyDifficult]

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

[HardlyDifficult]

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

[HardlyDifficult]

Some decent suggestions, incl the merged reports