Sablier
Sablier
DeFiFoundry
53,440 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

The function `SablierV2NFTDescriptor::calculateDurationInDays` does not verify if the `startTime` provided by user is strictly less than `endTime`, can cause underflow in `durationInDays`.

ctrus

Summary

The function SablierV2NFTDescriptor::calculateDurationInDays does not verify if the startTime provided by user is strictly less than endTime, causing underflow in the variable durationInDays when the provided startTime is greater than endTime. Which ultimately calculates the stream's duration in days incorrectly.

Vulnerability Details

Across the codebase, the protocol is usually checking for the inputs being provided by users are correct by employing relavent checks. This check is missing in SablierV2NFTDescriptors calculateDurationInDays function. The function does'nt check whether the startTime provided by a user is in past and is less than provided endTime. A user can provide inputs such that the startTime is greater than endTime causing underflow in durationInDays variable which goes unchecked for overflows and underflows as its wrapped inside unchecked keyword. ultimately calculates the stream's duration in days incorrectly.

///@audit underflow in durationInDays
unchecked {
durationInDays = (endTime - startTime) / 1 days;
}

Impact

A malicious user can provide timestamps such that the startTime is in future and is greater than endTime and this goes unnoticed as the function lacks a check. when the startTime is greater than endTime underflow occurs in durationInDays as its inside unchecked block, because solidity skips those safety checks for underflows and overflows when we use unchecked keyoword. this underflow can cause significant precision loss and resulting in calculating the streams duration incorrectly. and this wrong duration will be used to generate svg and ultimately tokenURI. everything will be calculated incorrectly due to this small missing check.

///The svg then generated would be wrong ultimately making the whole tokenURI wrong.
vars.svg = NFTSVG.generateSVG(
NFTSVG.SVGParams({
accentColor: generateAccentColor(address(sablier), streamId),
amount: abbreviateAmount({ amount: vars.depositedAmount, decimals: safeAssetDecimals(vars.asset) }),
assetAddress: vars.asset.toHexString(),
assetSymbol: vars.assetSymbol,
@> duration: calculateDurationInDays({
startTime: vars.sablier.getStartTime(streamId),
endTime: vars.sablier.getEndTime(streamId)
}),
sablierAddress: vars.sablierStringified,
progress: stringifyPercentage(vars.streamedPercentage),
progressNumerical: vars.streamedPercentage,
status: vars.status,
sablierModel: vars.sablierModel
})
);
​

Tools Used

Manual analysis

Recommendations

1.employ a check for the timestamps in the beginning of the function, check the startTime is less than endTime, if not the function should revert throwing an error. make an custom error and put it inside the check, which is thrown when startTime is not less than endTime.

function calculateDurationInDays(uint256 startTime, uint256 endTime) internal pure returns (string memory) {
+ if(startTime>endTime){
+ //custom error telling start time can't be greater than end time.
+ }
uint256 durationInDays;
unchecked {
durationInDays = (endTime - startTime) / 1 days;
}
​
// Return dummy values when the duration is either very small or very big.
if (durationInDays == 0) {
return string.concat(SVGElements.SIGN_LT, " 1 Day");
} else if (durationInDays > 9999) {
return string.concat(SVGElements.SIGN_GT, " 9999 Days");
}
​
string memory suffix = durationInDays == 1 ? " Day" : " Days";
return string.concat(durationInDays.toString(), suffix);
}

2.remove unchecked block, if its not that important.

Updates

Lead Judging Commences

inallhonesty Lead Judge
about 1 year ago
inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.