JPEG'd contest - Meta0xNull's results

Lines of code

https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/lock/JPEGLock.sol#L44-L62

Vulnerability details

Impact

1. Alice Request DAO to Call setNFTTypeValueETH() and DAO Approved it
2. Alice Call finalizePendingNFTValueETH() and External Function jpegLocker.lockFor was Called.
3. In JPEGLock.sol, Alice Transfer _lockAmount = 1000 of JPEGS into the Contract and info was saved in storage as lockAmount.

jpeg.safeTransferFrom(_account, address(this), _lockAmount);
positions[_nftIndex] = LockPosition({
            owner: _account,
            unlockAt: block.timestamp + lockTime,
            lockAmount: _lockAmount
        });

4. Alice Repeat Step 1 to 3 again and Transfer _lockAmount = 500 second time. It overwrite previous lockAmount = 1000 and now lockAmount = 500.
5. Once the lock period ended, Alice Call unlock() and Expecting 1500 JPEGS will Transfer to her wallet but Only Receive 500 JPEGS.

The first 1000 JPEGS Alice Transfer will be Permanently Lock into Contract.

Proof of Concept

https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/vaults/NFTVault.sol#L360-L375 https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/lock/JPEGLock.sol#L44-L62 https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/lock/JPEGLock.sol#L68-L77

Recommended Mitigation Steps

1. Check if the _nftIndex got any LockPosition.
2. If Yes, should add New _lockAmount into Existing lockAmount rather than overwrite it.

1) Long Revert Strings are Waste of Gas

Impact

Shortening revert strings to fit in 32 bytes will decrease deployment time gas and will decrease runtime gas when the revert condition has been met.

Revert strings that are longer than 32 bytes require at least one additional mstore, along with additional overhead for computing memory offset, etc.

Proof of Concept

https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/vaults/NFTVault.sol#L394 https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/tokens/StableCoin.sol#L41 https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/tokens/StableCoin.sol#L69

Recommended Mitigation Steps

Shorten the revert strings to fit in 32 bytes.

Or consider using Custom Errors (solc >=0.8.4).