Overlay Protocol contest - ye0lde's results

Handle

ye0lde

Vulnerability details

Impact

Open TODOs can point to architecture or programming issues that still need to be resolved.

Proof of Concept

The TODOs are here: https://github.com/code-423n4/2021-11-overlay/blob/1833b792caf3eb8756b1ba5f50f9c2ce085e54d0/contracts/collateral/OverlayV1OVLCollateral.sol#L304 https://github.com/code-423n4/2021-11-overlay/blob/1833b792caf3eb8756b1ba5f50f9c2ce085e54d0/contracts/collateral/OverlayV1OVLCollateral.sol#L396 https://github.com/code-423n4/2021-11-overlay/blob/1833b792caf3eb8756b1ba5f50f9c2ce085e54d0/contracts/market/OverlayV1OI.sol#L108

Tools Used

VS Code

Recommended Mitigation Steps

Consider resolving the TODOs before deploying.

Handle

ye0lde

Vulnerability details

Impact

Removing unused named return variables can reduce gas usage and improve code clarity.

Proof of Concept

Unused named returns https://github.com/code-423n4/2021-11-overlay/blob/1833b792caf3eb8756b1ba5f50f9c2ce085e54d0/contracts/market/OverlayV1PricePoint.sol#L142-L144

Tools Used

Visual Studio Code, Remix

Recommended Mitigation Steps

Remove the named return variables

Handle

ye0lde

Vulnerability details

Impact

Removing unneeded variables and code can reduce gas usage and improve code clarity.

Proof of Concept

The local variable _impact is defined and set but then copied to the named return variable impact_. They are both then used but are the same value at that point. https://github.com/code-423n4/2021-11-overlay/blob/1833b792caf3eb8756b1ba5f50f9c2ce085e54d0/contracts/market/OverlayV1Market.sol#L58-L66

Tools Used

Visual Studio Code, Remix

Recommended Mitigation Steps

Change line 58 - impact_ = intake(_isLong, _oi, _cap); Delete line 62 - impact_ = _impact; Change line 66 - collateralAdjusted_ = _collateral - impact_ - fee_;

Handle

ye0lde

Vulnerability details

Impact

Redundant arithmetic underflow/overflow checks can be avoided when an underflow/overflow cannot happen.

Proof of Concept

The "unchecked" keyword can be applied here since there is a "require" statement before to ensure the arithmetic operations would not cause an integer underflow or overflow. https://github.com/code-423n4/2021-11-overlay/blob/1833b792caf3eb8756b1ba5f50f9c2ce085e54d0/contracts/market/OverlayV1Market.sol#L66

Change the code at 66 to:

        unchecked { collateralAdjusted_ = _collateral - _impact - fee_; } 

Tools Used

Visual Studio Code, Remix

Recommended Mitigation Steps

Add the "unchecked" keyword as shown above.

I just realized this finding overlaps this previous finding: Unneeded variable and code in enterOI (OverlayV1Market.sol)

This mitigation is best applied after the mitigation for "Unneeded variable and code in enterOI"

        unchecked { collateralAdjusted_ = _collateral - impact_ - fee_; } 

Sorry wasn't trying to pad the findings, just found them at different times.

Handle

ye0lde

Vulnerability details

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/2021-11-overlay/blob/1833b792caf3eb8756b1ba5f50f9c2ce085e54d0/contracts/ovl/OverlayToken.sol#L131 https://github.com/code-423n4/2021-11-overlay/blob/1833b792caf3eb8756b1ba5f50f9c2ce085e54d0/contracts/ovl/OverlayToken.sol#L277 https://github.com/code-423n4/2021-11-overlay/blob/1833b792caf3eb8756b1ba5f50f9c2ce085e54d0/contracts/ovl/OverlayToken.sol#L310 https://github.com/code-423n4/2021-11-overlay/blob/1833b792caf3eb8756b1ba5f50f9c2ce085e54d0/contracts/ovl/OverlayToken.sol#L322-L328 https://github.com/code-423n4/2021-11-overlay/blob/1833b792caf3eb8756b1ba5f50f9c2ce085e54d0/contracts/ovl/OverlayToken.sol#L386 https://github.com/code-423n4/2021-11-overlay/blob/1833b792caf3eb8756b1ba5f50f9c2ce085e54d0/contracts/ovl/OverlayToken.sol#L403-L404

Tools Used

Visual Studio Code

Recommended Mitigation Steps

Consider shortening the revert strings to fit in 32 bytes or using custom errors (v0.8.4) in the future.

Handle

ye0lde

Vulnerability details

Impact

Code clarity

Proof of Concept

"@param" should be "@return" https://github.com/code-423n4/2021-11-overlay/blob/1833b792caf3eb8756b1ba5f50f9c2ce085e54d0/contracts/market/OverlayV1Market.sol#L83-L85

Not sure what this comment is for maybe just needs to be deleted. https://github.com/code-423n4/2021-11-overlay/blob/1833b792caf3eb8756b1ba5f50f9c2ce085e54d0/contracts/mothership/OverlayV1Mothership.sol#L155

Tools Used

Visual Studio Code, Remix

Recommended Mitigation Steps

Correct the comments if the suggestions are valid.