Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

QA Report #169

Open
code423n4 opened this issue Jul 3, 2022 · 0 comments
Open

QA Report #169

code423n4 opened this issue Jul 3, 2022 · 0 comments
Labels
bug Something isn't working QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax

Comments

@code423n4
Copy link
Contributor

Low Risk

[L-01] Unused receive() function will lock Ether in contract (If the intention is for the Ether to be used, the function should call another function, otherwise it should revert):-

  1. File: 2022-06-putty/contracts/lib/solmate/src/test/WETH.t.sol (line 107):

      `receive() external payable {}`

  2. File: 2022-06-putty/contracts/lib/solmate/src/test/WETH.t.sol (line 145):

      `receive() external payable {}`          

  3. File: 2022-06-putty/contracts/lib/solmate/src/tokens/WETH.sol (line 32):

      `receive() external payable virtual {`

[L-02] Missing checks for address(0x0) when assigning values to address state variables:-

  1. File: 2022-06-putty/contracts/lib/solmate/src/auth/Auth.sol (line 49):

      `owner = newOwner;`

  2. File: 2022-06-putty/contracts/lib/solmate/src/auth/Owned.sol (line 40):

      `owner = newOwner;`          

[L-03] Cross-chain replay attacks (Storing the block.chainid is not safe.):-

  1. File: 2022-06-putty/contracts/lib/solmate/src/tokens/ERC20.sol (line 168-174):

      `  keccak256(
            abi.encode(
                keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"),
                keccak256(bytes(name)),
                keccak256("1"),
                block.chainid,
                address(this)`

  2. File: 2022-06-putty/contracts/lib/solmate/src/test/WETH.t.sol (line 145):

      `receive() external payable {}`          

  3. File: 2022-06-putty/contracts/lib/solmate/src/tokens/WETH.sol (line 32):

      `receive() external payable virtual {`

Non-Critical Issues

[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(<str>,<str>)) :-

  1. File: 2022-06-putty/contracts/lib/solmate/src/test/CREATE3.t.sol (line 2):

      `pragma solidity 0.8.10;`

  2. File: 2022-06-putty/contracts/lib/solmate/src/test/ERC1155.t.sol (line 2):

      `pragma solidity 0.8.10;`          

  3. File: 2022-06-putty/contracts/lib/solmate/src/test/ERC20.t.sol (line 2):

      `pragma solidity 0.8.10;`

  4. File: 2022-06-putty/contracts/lib/solmate/src/test/ERC721.t.sol (line 2):

      `pragma solidity 0.8.10;`

  5. File: 2022-06-putty/contracts/lib/solmate/src/test/utils/DSTestPlus.sol (line 2):

      `pragma solidity 0.8.0;`          

  6. File: 2022-06-putty/contracts/lib/solmate/src/tokens/ERC20.sol (line 2):

      `pragma solidity 0.8.0;`               

  7. File: 2022-06-putty/contracts/lib/solmate/src/utils/CREATE3.sol (line 2):

      `pragma solidity 0.8.0;`

  8. File: 2022-06-putty/contracts/lib/solmate/src/utils/SSTORE2.sol (line 2):

      `pragma solidity 0.8.0;`    

[N-02] Event is missing indexed fields (Each event should use three indexed fields if there are three or more fields) :-

  1. File: 2022-06-putty/contracts/lib/solmate/src/mixins/ERC4626.sol (line 18-26):

      `event Deposit(address indexed caller, address indexed owner, uint256 assets, uint256 shares);

event Withdraw(
    address indexed caller,
    address indexed receiver,
    address indexed owner,
    uint256 assets,
    uint256 shares
);`

  2. File: 2022-06-putty/contracts/lib/solmate/src/test/utils/weird-tokens/MissingReturnToken.sol (line 9-11):

      `    event Transfer(address indexed from, address indexed to, uint256 amount);

event Approval(address indexed owner, address indexed spender, uint256 amount);`          

  3. File: 2022-06-putty/contracts/lib/solmate/src/test/utils/weird-tokens/ReturnsFalseToken.sol (line 9-11):

      `    event Transfer(address indexed from, address indexed to, uint256 amount);

event Approval(address indexed owner, address indexed spender, uint256 amount);`

  4. File: 2022-06-putty/contracts/lib/solmate/src/test/utils/weird-tokens/ReturnsGarbageToken.sol (line 9-11):

      `    event Transfer(address indexed from, address indexed to, uint256 amount);

event Approval(address indexed owner, address indexed spender, uint256 amount);`

  5. File: 2022-06-putty/contracts/lib/solmate/src/test/utils/weird-tokens/ReturnsTooLittleToken.sol (line 9-11):

      `    event Transfer(address indexed from, address indexed to, uint256 amount);

event Approval(address indexed owner, address indexed spender, uint256 amount);`          

  6. File: 2022-06-putty/contracts/lib/solmate/src/test/utils/weird-tokens/ReturnsTooMuchToken.sol (line 9-11):

      `event Transfer(address indexed from, address indexed to, uint256 amount);

event Approval(address indexed owner, address indexed spender, uint256 amount);`               

  7. File: 2022-06-putty/contracts/lib/solmate/src/test/utils/weird-tokens/ReturnsTwoToken.sol (line 9-11):

      `event Transfer(address indexed from, address indexed to, uint256 amount);

event Approval(address indexed owner, address indexed spender, uint256 amount);`

  8. File: 2022-06-putty/contracts/lib/solmate/src/test/utils/weird-tokens/RevertingToken.sol (line 9-11):

      `event Transfer(address indexed from, address indexed to, uint256 amount);

event Approval(address indexed owner, address indexed spender, uint256 amount);`              

  9. File: 2022-06-putty/contracts/lib/solmate/src/tokens/ERC1155.sol (line 11-29):

      `    event TransferSingle(
    address indexed operator,
    address indexed from,
    address indexed to,
    uint256 id,
    uint256 amount
);

event TransferBatch(
    address indexed operator,
    address indexed from,
    address indexed to,
    uint256[] ids,
    uint256[] amounts
);

event ApprovalForAll(address indexed owner, address indexed operator, bool approved);

event URI(string value, uint256 indexed id);`          

  10. File: 2022-06-putty/contracts/lib/solmate/src/tokens/ERC20.sol (line 13-15):

      `event Transfer(address indexed from, address indexed to, uint256 amount);

event Approval(address indexed owner, address indexed spender, uint256 amount);`               

  11. File: 2022-06-putty/contracts/lib/solmate/src/tokens/WETH.sol (line 14-16):

      `event Deposit(address indexed from, uint256 amount);

event Withdrawal(address indexed to, uint256 amount);`     

[N-03] Multiple address mappings can be combined into a single mapping of an address to a struct, where appropriate :-

  1. File: 2022-06-putty/contracts/lib/solmate/src/auth/authorities/MultiRolesAuthority.sol (line 31-41):

      `mapping(address => Authority) public getTargetCustomAuthority;

/*//////////////////////////////////////////////////////////////
                        ROLE/USER STORAGE
//////////////////////////////////////////////////////////////*/

mapping(address => bytes32) public getUserRoles;

mapping(bytes4 => bool) public isCapabilityPublic;

mapping(bytes4 => bytes32) public getRolesWithCapability;`

  2. File: 2022-06-putty/contracts/lib/solmate/src/auth/authorities/RolesAuthority.sol (line 30-34):

      ` mapping(address => bytes32) public getUserRoles;

mapping(address => mapping(bytes4 => bool)) public isCapabilityPublic;

mapping(address => mapping(bytes4 => bytes32)) public getRolesWithCapability;`          

  3. File: 22022-06-putty/contracts/lib/solmate/src/test/ERC1155.t.sol (line 114-115):

      `mapping(address => mapping(uint256 => uint256)) public userMintAmounts;
mapping(address => mapping(uint256 => uint256)) public userTransferOrBurnAmounts;`
@code423n4 code423n4 added bug Something isn't working QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax labels Jul 3, 2022
code423n4 added a commit that referenced this issue Jul 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax
Projects
None yet
Development

No branches or pull requests

1 participant