In the GSP contract's init function, there is an inconsistency between the comment and the actual check for the k parameter. The comment suggests that k should be greater than 0 and less than 1e18. However, the implemented check only ensures k is less than or equal to 1e18, allowing k to be 0 or 10**18. This discrepancy could lead to unintended behavior or vulnerabilities in the contract's functionality especially in the library PMMPricing where k is critical for the getMidPrice function and the library DODOMath where k is critically important.
Impact
Allowing k to be 0 or 10e18, despite the comment suggesting otherwise, might lead to scenarios not accounted for in the contract logic.
Code Snippet
...
contract GSP is GSPTrader, GSPFunding {
...
function init(
address maintainer,
address baseTokenAddress,
address quoteTokenAddress,
uint256 lpFeeRate,
uint256 mtFeeRate,
uint256 i,
uint256 k,
bool isOpenTWAP
) external {
...
// k should be greater than 0 and less than 10**18
require(k <= 10**18);
_K_ = k;
...
}
Tool Used
Manual Review
Recommendation
Ensure consistency between the comment and the implemented logic. If k is intended to be strictly greater than 0 and less than 1e18, update the require statement to reflect this. Consider revising the comment if the current implementation aligns with the intended functionality. Additionally, evaluate the implications of k being 0 or 1e18 in the contract's logic and address potential vulnerabilities or logical errors that might arise from these edge cases.
dian.ivanov
high
K can be 0 or 1e18
dian.ivanov
high
K can be 0 or 1e18
Summary
Inconsistency and Potential Issue in GSP Contract's Initialization of Parameter K: https://github.com/sherlock-audit/2023-12-dodo-gsp/blob/af43d39f6a89e5084843e196fc0185abffe6304d/dodo-gassaving-pool/contracts/GasSavingPool/impl/GSP.sol#L57
Vulnerability Detail
In the GSP contract's init function, there is an inconsistency between the comment and the actual check for the k parameter. The comment suggests that k should be greater than 0 and less than 1e18. However, the implemented check only ensures k is less than or equal to 1e18, allowing k to be 0 or 10**18. This discrepancy could lead to unintended behavior or vulnerabilities in the contract's functionality especially in the library PMMPricing where k is critical for the getMidPrice function and the library DODOMath where k is critically important.
Impact
Allowing k to be 0 or 10e18, despite the comment suggesting otherwise, might lead to scenarios not accounted for in the contract logic.
Code Snippet
Tool Used
Manual Review
Recommendation
Ensure consistency between the comment and the implemented logic. If k is intended to be strictly greater than 0 and less than 1e18, update the require statement to reflect this. Consider revising the comment if the current implementation aligns with the intended functionality. Additionally, evaluate the implications of k being 0 or 1e18 in the contract's logic and address potential vulnerabilities or logical errors that might arise from these edge cases.
Duplicate of #76