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 #92

Open
code423n4 opened this issue Apr 21, 2022 · 0 comments
Open

QA Report #92

code423n4 opened this issue Apr 21, 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

QA Report

Table of Contents

summary

Few vulnerabilities were found examining the contracts. The main concerns are with standard token methods such as transfer(), mint() or burn() not being implemented properly

The presence of an assert statement was also found, which is bad practice for functions that can be called by external users.

Most of the contracts are using uint as an alias for uint256. For readability, it is best practice to use uint256

transfer function should return a boolean

IMPACT

transfer() and transferFrom() should return a boolean. It can be confusing when some functions are expected to return something and others are silent.

SEVERITY

Low

PROOF OF CONCEPT

Instances include:

vToken.sol

vToken.sol:76
vToken.sol:81

IvToken.sol

IvToken.sol:24
IvToken.sol:48

TOOLS USED

Manual Analysis

MITIGATION

Add a return boolean to these functions

assert statement should not be used

IMPACT

Properly functioning code should never reach a failing assert statement. If it happened, it would indicate the presence of a bug in the contract. A failing assert uses all the remaining gas, which can be financially painful for a user.

SEVERITY

Low

PROOF OF CONCEPT

Instances include:

IndexLogic.sol

IndexLogic.sol:72: assert(minAmountInBase != type(uint).max);

TOOLS USED

Manual Analysis

MITIGATION

Replace the assert statement with a require statement or a custom error

mint and burn missing zero address check

IMPACT

mint() and burn() should check that _recipient is not the zero address.

SEVERITY

Low

PROOF OF CONCEPT

Instances include:

BaseIndex.sol

BaseIndex.sol:43
BaseIndex.sol:59

TOOLS USED

Manual Analysis

MITIGATION

Add a zero address check before performing the delegatecall

Uint256 alias

IMPACT

uint is an alias for uint256.

It is better to use uint256: it brings readability and consistency in the code, and it future proofs it in case of any changes to the alias of uint

SEVERITY

Non-Critical

PROOF OF CONCEPT

Instances include:

BaseIndex.sol

BaseIndex.sol:78:    uint i

AssetInfo.sol

AssetInfo.sol:25:    uint lastAssetPerBaseInUQ
AssetInfo.sol:25:    uint assetPerBaseInUQ

IndexLayout.sol

IndexLayout.sol:20:    uint internal lastTransferTime

IndexLogic.sol

IndexLogic.sol:37:    uint lastAssetBalanceInBase
IndexLogic.sol:38:    uint minAmountInBase
IndexLogic.sol:39:    uint i
IndexLogic.sol:44:    uint assetPerBaseInUQ
IndexLogic.sol:48:    uint amountInAsset
IndexLogic.sol:49:    uint weightedPrice
IndexLogic.sol:50:    uint _minAmountInBase
IndexLogic.sol:54:    uint lastBalanceInAsset
IndexLogic.sol:56:    uint balanceInBase
IndexLogic.sol:60:    uint i
IndexLogic.sol:62:    uint lastBalanceInAsset
IndexLogic.sol:74:    uint value
IndexLogic.sol:85:    uint fee
IndexLogic.sol:97:    uint value
IndexLogic.sol:99:    uint length
IndexLogic.sol:102:   uint i
IndexLogic.sol:112:   uint fee
IndexLogic.sol:124:   uint lastOrderId
IndexLogic.sol:125:   uint i
IndexLogic.sol:132:   uint indexAssetBalance
IndexLogic.sol:133:   uint accountBalance

ManagedIndex.sol

ManagedIndex.sol:30:    uint i

ManagedIndexReweightingLogic.sol

ManagedIndexReweightingLogic.sol:36:    uint virtualEvaluationInBase
ManagedIndexReweightingLogic.sol:38:    uint i
ManagedIndexReweightingLogic.sol:39:    uint priceAssetPerBaseInUQ
ManagedIndexReweightingLogic.sol:40:    uint availableAssets
ManagedIndexReweightingLogic.sol:46:    uint orderId
ManagedIndexReweightingLogic.sol:48:    uint _totalWeight
ManagedIndexReweightingLogic.sol:50:    uint i
ManagedIndexReweightingLogic.sol:74:    uint amountInBase
ManagedIndexReweightingLogic.sol:75:    uint amountInAsset
ManagedIndexReweightingLogic.sol:76:    uint newShares
ManagedIndexReweightingLogic.sol:76:    uint oldShares
ManagedIndexReweightingLogic.sol:96:    uint i
ManagedIndexReweightingLogic.sol:97:    uint shares

PhutureIndex.sol

PhutureIndex.sol:47:    uint _value
PhutureIndex.sol:56:    uint timePassed
PhutureIndex.sol:58:    uint fee

PhuturePriceOracle.sol

PhuturePriceOracle.sol:68:    uint _baseAmount

TopNMarketCapIndex.sol

TopNMarketCapIndex.sol:22:    uint public category
TopNMarketCapIndex.sol:23:    uint public snapshot
TopNMarketCapIndex.sol:39:    uint _category
TopNMarketCapIndex.sol:40:    uint _snapshot
TopNMarketCapIndex.sol:43:    uint _totalCapitalization
TopNMarketCapIndex.sol:48:    uint i
TopNMarketCapIndex.sol:49:    uint _i

TopNMarketCapIndexReweightingLogic.sol

TopNMarketCapIndex.sol:31:    uint _category
TopNMarketCapIndex.sol:32:    uint _snapshot
TopNMarketCapIndex.sol:33:    uint _topN
TopNMarketCapIndex.sol:36:    uint virtualEvaluationInBase
TopNMarketCapIndex.sol:37:    uint i
TopNMarketCapIndex.sol:38:    uint priceAssetPerBaseInUQ
TopNMarketCapIndex.sol:39:    uint availableAssets
TopNMarketCapIndex.sol:48:    uint orderId
TopNMarketCapIndex.sol:51:    uint _i
TopNMarketCapIndex.sol:52:    uint i
TopNMarketCapIndex.sol:57:    uint shares
TopNMarketCapIndex.sol:89:    uint amountInBase
TopNMarketCapIndex.sol:90:    uint amountInAsset
TopNMarketCapIndex.sol:94:    uint newShares
TopNMarketCapIndex.sol:94:    uint oldShares
TopNMarketCapIndex.sol:104:    uint i
TopNMarketCapIndex.sol:105:    uint shares

TrackedIndex.sol

TrackedIndex.sol:28:    uint _totalCapitalization
TrackedIndex.sol:33:    uint maxCapitalization
TrackedIndex.sol:35:    uint i

TrackedIndexReweightingLogic.sol

TrackedIndexReweightingLogic.sol:30:    uint _totalCapitalization
TrackedIndexReweightingLogic.sol:33:    uint virtualEvaluationInBase
TrackedIndexReweightingLogic.sol:35:    uint maxCapitalization
TrackedIndexReweightingLogic.sol:37:    uint i
TrackedIndexReweightingLogic.sol:40:    uint priceAssetPerBaseInUQ
TrackedIndexReweightingLogic.sol:41:    uint availableAssets
TrackedIndexReweightingLogic.sol:64:    uint orderId
TrackedIndexReweightingLogic.sol:66:    uint i
TrackedIndexReweightingLogic.sol:68:    uint amountInBase
TrackedIndexReweightingLogic.sol:69:    uint amountInAsset
TrackedIndexReweightingLogic.sol:70:    uint newShares
TrackedIndexReweightingLogic.sol:70:    uint oldShares

UniswapV2PathPriceOracle.sol

UniswapV2PathPriceOracle.sol:32:    uint currentAssetPerBaseInUQ
UniswapV2PathPriceOracle.sol:34:    uint i
UniswapV2PathPriceOracle.sol:47:    uint currentAssetPerBaseInUQ
UniswapV2PathPriceOracle.sol:49:    uint i

UniswapV2PriceOracle.sol

UniswapV2PriceOracle.sol:19:    uint private constant MIN_UPDATE_INTERVAL = 24 hours;
UniswapV2PriceOracle.sol:27:    uint private price0CumulativeLast;
UniswapV2PriceOracle.sol:28:    uint private price1CumulativeLast;
UniswapV2PriceOracle.sol:30:    uint private price0Average;
UniswapV2PriceOracle.sol:31:    uint private price1Average;
UniswapV2PriceOracle.sol:48:    uint _price0CumulativeLast;
UniswapV2PriceOracle.sol:49:    uint _price1CumulativeLast;
UniswapV2PriceOracle.sol:50:    uint price0Cml;
UniswapV2PriceOracle.sol:50:    uint price1Cml;
UniswapV2PriceOracle.sol:62:    uint price0Cumulative;
UniswapV2PriceOracle.sol:62:    uint price1Cumulative;

vToken.sol

vToken.sol:70:    uint _amount
vToken.sol:76:    uint _amount
vToken.sol:84:    uint _shares
vToken.sol:90:    uint shares
vToken.sol:95:    uint _amount
vToken.sol:125:    uint _amount
vToken.sol:145:    uint _shares
vToken.sol:147:    uint amountInAsset
vToken.sol:152:    uint _amountInAsset
vToken.sol:156:    uint newShares
vToken.sol:156:    uint oldShares
vToken.sol:159:    uint _totalSupply
vToken.sol:161:    uint _balance
vToken.sol:162:    uint _assetBalance
vToken.sol:163:    uint availableAssets
vToken.sol:183:    uint shares
vToken.sol:184:    uint _totalAssetSupply
vToken.sol:193:    uint amount
vToken.sol:194:    uint shares
vToken.sol:208:    uint _amount
vToken.sol:213:    uint _amount
vToken.sol:218:    uint balance

TOOLS USED

Manual Analysis

MITIGATION

replace uint with
uint256

@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 Apr 21, 2022
code423n4 added a commit that referenced this issue Apr 21, 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