QA Report - Githubissues

code423n4 commented 2 years ago

[N-01] Use a more recent version of solidity Use a solidity version of at least 0.8.12 to get string.concat() to be used instead of abi.encodePacked(,)

[N-02] Lock pragmas to specific compiler version Contracts should be deployed with the same compiler version and flags that they have been tested with thoroughly. Locking the pragma helps to ensure that contracts do not accidentally get deployed using, for example, an outdated compiler version that might introduce bugs that affect the contract system negatively. see https://swcregistry.io/docs/SWC-103

[N-03] Namings are not appropriate (1 letter)

Instances include: https://github.com/Plex-Engineer/lending-market-v2/blob/443a8c0fed3c5018e95f3881a31b81a555c42b2d/contracts/Stableswap/BaseV1-periphery.sol#L350 https://github.com/Plex-Engineer/lending-market-v2/blob/443a8c0fed3c5018e95f3881a31b81a555c42b2d/contracts/Stableswap/BaseV1-periphery.sol#L369

[N-04] Use of sensitive/non-inclusive terms

Instances include: https://github.com/Plex-Engineer/manifest-v2/blob/f6ebfe679973edf4f64832e64480ff5250ef8486/contracts/Proposal-Store.sol#L6

Mitigation: change 'evil' to 'unfriendly'

[L-01] abi.encodePacked should not be used with dynamic types when passing the result to a hash function such as keccak256()

Instances include: https://github.com/Plex-Engineer/lending-market-v2/blob/443a8c0fed3c5018e95f3881a31b81a555c42b2d/contracts/Stableswap/BaseV1-core.sol#L427 https://github.com/Plex-Engineer/lending-market-v2/blob/443a8c0fed3c5018e95f3881a31b81a555c42b2d/contracts/Stableswap/BaseV1-periphery.sol#L539

Mitigation: Use abi.encode() instead which will pad items to 32 bytes, which will prevent hash collisions (e.g. abi.encodePacked(0x123,0x456) => 0x123456 => abi.encodePacked(0x1,0x23456), but abi.encode(0x123,0x456) => 0x0...1230...456). “Unless there is a compelling reason, abi.encode should be preferred”. https://docs.soliditylang.org/en/v0.8.13/abi-spec.html#non-standard-packed-mode (in 'warning')

[L-02] require() should be used instead of assert()

Instances include: https://github.com/Plex-Engineer/lending-market-v2/blob/443a8c0fed3c5018e95f3881a31b81a555c42b2d/contracts/Stableswap/BaseV1-periphery.sol#L100 https://github.com/Plex-Engineer/lending-market-v2/blob/443a8c0fed3c5018e95f3881a31b81a555c42b2d/contracts/Stableswap/BaseV1-periphery.sol#L245 https://github.com/Plex-Engineer/lending-market-v2/blob/443a8c0fed3c5018e95f3881a31b81a555c42b2d/contracts/Stableswap/BaseV1-periphery.sol#L291 https://github.com/Plex-Engineer/lending-market-v2/blob/443a8c0fed3c5018e95f3881a31b81a555c42b2d/contracts/Stableswap/BaseV1-periphery.sol#L437 https://github.com/Plex-Engineer/lending-market-v2/blob/443a8c0fed3c5018e95f3881a31b81a555c42b2d/contracts/Comptroller.sol#L219 https://github.com/Plex-Engineer/lending-market-v2/blob/443a8c0fed3c5018e95f3881a31b81a555c42b2d/contracts/Comptroller.sol#L365

GalloDaSballo commented 2 years ago

[L-01] abi.encodePacked

Disputed, you must have a risk of clash for this finding to be valid. The day you find a clash, submit as higher severity

[L-02] require() should be used instead of assert()

Valid Low

GalloDaSballo commented 2 years ago

[N-01] Use a more recent version of solidity

NC

[N-02] Lock pragmas to specific compiler version

NC

[N-03] Namings are not appropriate (1 letter)

Disagree given the widespread terminology for signature parts

[N-04] Use of sensitive/non-inclusive terms

Disputed, evil is perfectly adequate as a word (the fact the comment exists is technically a mistake, but that seems to have been missed), that said, thank you for the chuckle

GalloDaSballo commented 2 years ago

1L 2NC

code-423n4 / 2022-06-canto-v2-findings

QA Report #170

[L-01] abi.encodePacked

[L-02] require() should be used instead of assert()

[N-01] Use a more recent version of solidity

[N-02] Lock pragmas to specific compiler version

[N-03] Namings are not appropriate (1 letter)

[N-04] Use of sensitive/non-inclusive terms