Canto contest - sach1r0's results

Execution layer for original work.

General Information

Platform: Code4rena

Start Date: 14/06/2022

Pot Size: $100,000 USDC

Total HM: 26

Participants: 59

Period: 7 days

Judge: GalloDaSballo

Total Solo HM: 9

Id: 133

League: ETH

Canto

Findings Distribution

Researcher Performance

Rank: 40/59

Findings: 2

Award: $290.75

🌟 Selected for report: 0

🚀 Solo Findings: 0

External Links

Code4rena Contest Page Github Contest Repository Github Findings Repository Canto

Findings Information

🌟 Selected for report: joestakey

Also found by: 0x1f8b, 0x29A, 0x52, 0xDjango, 0xNazgul, 0xf15ers, 0xmint, Bronicle, Dravee, Funen, JMukesh, Limbooo, MadWookie, Picodes, Ruhum, TerrierLover, TomJ, Tutturu, WatchPug, Waze, _Adam, asutorufos, c3phas, catchup, cccz, codexploder, cryptphi, csanuragjain, defsec, fatherOfBlocks, gzeon, hake, hansfriese, hyh, ignacio, k, nxrblsrpr, oyc_109, robee, sach1r0, saian, simon135, technicallyty, zzzitron

Awards

74.2759 USDC - $74.28

687.9945 CANTO - $111.11

Labels

bug

QA (Quality Assurance)

External Links

Link to GitHub issue

Some functions does not check for zero address

Details

It's important to check for zero-address to avoid redeploying of the contract when the address is accidentally set to zero-address.

Mitigation

Add a require statement, for example: require(_pauser != address(0));

Line of code

https://github.com/Plex-Engineer/stableswap/blob/489d010eb99a0885139b2d5ed5a2d826838cc5f9/contracts/BaseV1-core.sol#L497-L500 https://github.com/Plex-Engineer/lending-market/blob/755424c1f9ab3f9f0408443e6606f94e4f08a990/contracts/Governance/GovernorBravoDelegate.sol#L131-L137 https://github.com/Plex-Engineer/lending-market/blob/755424c1f9ab3f9f0408443e6606f94e4f08a990/contracts/Governance/GovernorBravoDelegate.sol#L144-L156 https://github.com/Plex-Engineer/lending-market/blob/755424c1f9ab3f9f0408443e6606f94e4f08a990/contracts/Treasury/TreasuryDelegator.sol#L63-L65 https://github.com/Plex-Engineer/lending-market/blob/755424c1f9ab3f9f0408443e6606f94e4f08a990/contracts/Treasury/TreasuryDelegate.sol#L46-L58

Lack of zero-address check in the constructor

Details

Lack of zero-address checks may lead to infunctional protocol especially in the case wherein variable is immutable like the factory .

Mitigation

Consider adding zero-address checks such as: require(_factory != address(0));

Line of code:

https://github.com/Plex-Engineer/stableswap/blob/489d010eb99a0885139b2d5ed5a2d826838cc5f9/contracts/BaseV1-periphery.sol#L75-L79 https://github.com/Plex-Engineer/lending-market/blob/755424c1f9ab3f9f0408443e6606f94e4f08a990/contracts/Treasury/TreasuryDelegator.sol#L12-L22

Using `assert` instead of `require`

Details/POC

According to the solidity docs[https://docs.soliditylang.org/en/latest/control-structures.html?highlight=assert#panic-via-assert-and-error-via-require]: Assert should only be used to test for internal errors, and to check invariants. Properly functioning code should never create a Panic, not even on invalid external input. If this happens, then there is a bug in your contract which you should fix. see reference: https://github.com/code-423n4/2022-01-openleverage-findings/issues/43

Impact

An assert will consume all gas of the transaction whereas a revert/require releases the remaining gas to the transaction sender again. Usually, one wants to try to keep the gas cost for contract failures low and use assert only for invariants that should always be true.

Mitigation

Use require instead of assert

Line of code:

https://github.com/Plex-Engineer/stableswap/blob/489d010eb99a0885139b2d5ed5a2d826838cc5f9/contracts/BaseV1-periphery.sol#L82 https://github.com/Plex-Engineer/stableswap/blob/489d010eb99a0885139b2d5ed5a2d826838cc5f9/contracts/BaseV1-periphery.sol#L273 https://github.com/Plex-Engineer/lending-market/blob/755424c1f9ab3f9f0408443e6606f94e4f08a990/contracts/Comptroller.sol#L214

Non-Library/Interface files should use fixed compiler versions, not floating ones

Details

Contracts should be deployed with the same compiler version/flags where they have been tested with. Locking the pragma helps to ensure that contracts do not accidentally get deployed using an outdated compiler version that might introduce bugs that affect the contract system negatively. see reference: https://github.com/code-423n4/2021-11-unlock-findings/issues/15, https://code4rena.com/reports/2022-03-paladin/ and https://swcregistry.io/docs/SWC-103

Mitigation

I suggest changing pragma solidity ^0.8.10; to pragma solidity 0.8.11;

Line of code:

Use of `pragma experimental ABIEncoderV2`

Details

According to the solidity documentation (https://docs.soliditylang.org/en/v0.8.13/080-breaking-changes.html): The pragma pragma experimental ABIEncoderV2; is still valid, but it is deprecated and has no effect.

Mitigation

If you want to be explicit, please use pragma abicoder v2; instead.

Line of code

https://github.com/Plex-Engineer/lending-market/blob/755424c1f9ab3f9f0408443e6606f94e4f08a990/contracts/Accountant/AccountantDelegator.sol#L2

Missing SPDX-License-Identifier

Details

The following contracts are missing SPDX-License-Identifier which correctly license the contract for open source development: AccountantDelegate.sol AccountantDelegator.sol AccountantInterfaces.sol TreasuryDelegator.sol TreasuryDelegate.sol TreasuryInterfaces.sol CNote.sol NoteInterest.sol WETH.sol

see reference: https://github.com/code-423n4/2021-09-sushimiso-findings/issues/26

Mitigation

I suggest adding // SPDX-License-Identifier: MIT to the top of the aforementioned files.

#0 - GalloDaSballo
2022-08-03T23:31:40Z

Some functions does not check for zero address

Using assert instead of require

Non-Library/Interface files should use fixed compiler versions, not floating ones

Use of pragma experimental ABIEncoderV2

Missing SPDX-License-Identifier

1L 1R 3NC

Findings Information

🌟 Selected for report: _Adam

Also found by: 0v3rf10w, 0x1f8b, 0x29A, 0xKitsune, 0xNazgul, 0xf15ers, 0xkatana, 0xmint, Chom, Dravee, Fitraldys, Funen, JC, Limbooo, MadWookie, Picodes, Ruhum, TerrierLover, TomJ, Tomio, Waze, ak1, c3phas, catchup, defsec, fatherOfBlocks, gzeon, hake, hansfriese, joestakey, k, oyc_109, rfa, robee, sach1r0, saian, simon135, ynnad

Awards

41.2642 USDC - $41.26

396.9199 CANTO - $64.10

Labels

bug

G (Gas Optimization)

External Links

Link to GitHub issue

No need to explicitly initialize variables with their default values

Details

When variables are not set, it is assumed to have it's default value(0 for uint, false for bool, address(0) for address). Explicitly initializing it with its default value is an anti-pattern and wastes gas.

Mitigation

change uint public totalSupply = 0; to uint public totalSupply; see reference: https://code4rena.com/reports/2022-02-jpyc/ [G-07] GENERAL RECOMMENDATIONS

Line of code:

Solidity compiler will always read the length of the array during each iteration

Details

.length in a loop can be extracted into a variable and used where necessary to reduce the number of storage reads see reference: https://github.com/code-423n4/2021-10-union-findings/issues/92

Mitigation:

This extra costs can be avoided by caching the array length. Example: uint _pricesLength = _prices.length; for (uint i = 0; i < pricesLength; i++) { }

Line of code:

Pre-increment cost less gas than post-increment

Details

i++ costs more gas than ++i , for uint pre-decrement is cheaper than post-decrement see reference: https://github.com/code-423n4/2021-12-nftx-findings/issues/195

Mitigation

change i++ to ++i

Line of code:

`!=` is cheaper in gas compared to `>` for uint

Details

!= 0 costs less gas compared to > 0 for unsigned integers in require statements with the optimizer enabled (6 gas) see reference: https://github.com/code-423n4/2021-12-maple-findings/issues/75

Mitigation

use != 0 instead of > 0

Line of code:

https://github.com/Plex-Engineer/stableswap/blob/489d010eb99a0885139b2d5ed5a2d826838cc5f9/contracts/BaseV1-core.sol#L465 https://github.com/Plex-Engineer/stableswap/blob/489d010eb99a0885139b2d5ed5a2d826838cc5f9/contracts/BaseV1-periphery.sol#L456 https://github.com/Plex-Engineer/stableswap/blob/489d010eb99a0885139b2d5ed5a2d826838cc5f9/contracts/BaseV1-periphery.sol#L463

Functions that are not called within the contract must set its visibility to `external` instead of `public`

Details

Setting function's visibility to external when it is only called externally can save gas because external function’s parameters are not copied into memory and are instead read from calldata directly. see reference: https://github.com/code-423n4/2021-06-gro-findings/issues/37

Mitigation

Set function visibility to external

Line of code:

#0 - GalloDaSballo
2022-08-04T00:41:08Z

Less than 500 gas

Canto contest - sach1r0's results

Execution layer for original work.

General Information

Findings Distribution

Researcher Performance

External Links

[Q-28] QA Report - $185.39

Findings Information

Awards

Labels

External Links

Some functions does not check for zero address

Details

Mitigation

Line of code

Lack of zero-address check in the constructor

Details

Mitigation

Line of code:

Using assert instead of require

Details/POC

Impact

Mitigation

Line of code:

Non-Library/Interface files should use fixed compiler versions, not floating ones

Details

Mitigation

Line of code:

Use of pragma experimental ABIEncoderV2

Details

Mitigation

Line of code

Missing SPDX-License-Identifier

Details

Mitigation

#0 - GalloDaSballo2022-08-03T23:31:40Z

Some functions does not check for zero address

Using assert instead of require

Non-Library/Interface files should use fixed compiler versions, not floating ones

Use of pragma experimental ABIEncoderV2

Missing SPDX-License-Identifier

[G-32] Gas Report - $105.36

Findings Information

Awards

Labels

External Links

No need to explicitly initialize variables with their default values

Details

Mitigation

Line of code:

Solidity compiler will always read the length of the array during each iteration

Details

Mitigation:

Line of code:

Pre-increment cost less gas than post-increment

Details

Mitigation

Line of code:

!= is cheaper in gas compared to > for uint

Details

Mitigation

Line of code:

Functions that are not called within the contract must set its visibility to external instead of public

Details

Mitigation

Line of code:

#0 - GalloDaSballo2022-08-04T00:41:08Z

Using `assert` instead of `require`

Use of `pragma experimental ABIEncoderV2`

#0 - GalloDaSballo
2022-08-03T23:31:40Z

`!=` is cheaper in gas compared to `>` for uint

Functions that are not called within the contract must set its visibility to `external` instead of `public`

#0 - GalloDaSballo
2022-08-04T00:41:08Z