search

code-423n4 / 2022-02-hubble-findings

2 stars 2 forks source link

QA Report #69

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

1. Use uint256 Instead of uint

Impact

Use uint256 because it is consistent with other uint data types, which also specify their size, and also because making the size of the data explicit reminds the developer and the reader how much data they've got to play with, which may help prevent or detect bugs.

It's also better to be explicit when constructing method signature ID's. For example if doing bytes4(keccak('transfer(address, uint)')), you'll get a different method sig ID than bytes4(keccak('transfer(address, uint256)')) and smart contracts will only understand the latter when comparing method sig IDs.

Proof of Concept

https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/VUSD.sol#L25 https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/VUSD.sol#L28 More...

2. TODOs List May Leak Important Info & Errors

Impact

Open TODOs can hint at programming or architectural errors that still need to be fixed.

Proof of Concept

https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/AMM.sol#L142 https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/AMM.sol#L555 https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/ClearingHouse.sol#L172 https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/MarginAccount.sol#L277

Recommended Mitigation Steps

Fix TODOs List and Remove it.

moose-code commented 2 years ago

Would be nice to understand the motivation to use uint as opposed to uint256. Am sure its been thought through.