Sablier

DeFiFoundry

53,440 USDC

View results

Previous Next

Submission Details

Severity: low

Invalid

Non Critical issues (11-20) of 73

mmm

NC011 - Consider using delete rather than assigning false/zero to clear values:

The delete keyword more closely matches the semantics of what is being done, and draws more attention to the changing of state, which may lead to a more thorough audit of its associated logic.

Click to show 4 findings

File: v2-core/src/SablierV2LockupDynamic.sol

103 lockupStream.isCancelable = false;

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/SablierV2LockupDynamic.sol#L0:0

File: v2-core/src/SablierV2LockupLinear.sol

88 lockupStream.isCancelable = false;

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/SablierV2LockupLinear.sol#L0:0

File: v2-core/src/SablierV2LockupTranched.sol

87 lockupStream.isCancelable = false;

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/SablierV2LockupTranched.sol#L0:0

File: v2-core/src/abstracts/SablierV2Lockup.sol

581 _streams[streamId].isCancelable = false;

627 _streams[streamId].isCancelable = false;

645 _streams[streamId].isCancelable = false;

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/abstracts/SablierV2Lockup.sol#L0:0

NC012 - Lines are too long:

Usually lines in source code are limited to 80 characters. Today's screens are much larger so it's reasonable to stretch this in some cases. The solidity style guide recommends a maximumum line length of 120 characters, so the lines below should be split when they reach that length.

Click to show 18 findings

File: v2-core/src/SablierV2LockupDynamic.sol

params.asset.safeTransferFrom({ from: msg.sender, to: params.broker.account, value: createAmounts.brokerFee });

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/SablierV2LockupDynamic.sol#L361:361

File: v2-core/src/SablierV2LockupLinear.sol

params.asset.safeTransferFrom({ from: msg.sender, to: params.broker.account, value: createAmounts.brokerFee });

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/SablierV2LockupLinear.sol#L281:281

File: v2-core/src/SablierV2LockupTranched.sol

timestamps = LockupTranched.Timestamps({ start: _streams[streamId].startTime, end: _streams[streamId].endTime });

params.asset.safeTransferFrom({ from: msg.sender, to: params.broker.account, value: createAmounts.brokerFee });

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/SablierV2LockupTranched.sol#L265:265

File: v2-core/src/SablierV2NFTDescriptor.sol

unicode"⚠️ WARNING: Transferring the NFT makes the new owner the recipient of the stream. The funds are not automatically withdrawn for the previous recipient."

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/SablierV2NFTDescriptor.sol#L276:276

File: v2-core/src/abstracts/SablierV2Lockup.sol

result = status == Lockup.Status.SETTLED || status == Lockup.Status.CANCELED || status == Lockup.Status.DEPLETED;

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/abstracts/SablierV2Lockup.sol#L151:151

NC013 - File is missing NatSpec comments:

The file does not contain any of the NatSpec comments (@inheritdoc, @param, @return, @notice), which are important for documentation and user confirmation.

File: v2-core/src/libraries/NFTSVG.sol

// SPDX-License-Identifier: GPL-3.0-or-later

// solhint-disable quotes

pragma solidity >=0.8.22;

import { Strings } from "hot/node_modules/@openzeppelin/contracts/utils/Strings.sol";

import { SVGElements } from "./SVGElements.sol";

library NFTSVG {

using Strings for uint256;

uint256 internal constant CARD_MARGIN = 16;

struct SVGParams {

string accentColor;

string amount;

string assetAddress;

string assetSymbol;

string duration;

string progress;

uint256 progressNumerical;

string sablierAddress;

string sablierModel;

string status;

}

struct SVGVars {

string amountCard;

uint256 amountWidth;

uint256 amountXPosition;

string cards;

uint256 cardsWidth;

string durationCard;

uint256 durationWidth;

uint256 durationXPosition;

string progressCard;

uint256 progressWidth;

uint256 progressXPosition;

string statusCard;

uint256 statusWidth;

uint256 statusXPosition;

}

function generateSVG(SVGParams memory params) internal pure returns (string memory) {

SVGVars memory vars;

// Generate the progress card.

(vars.progressWidth, vars.progressCard) = SVGElements.card({

cardType: SVGElements.CardType.PROGRESS,

content: params.progress,

circle: SVGElements.progressCircle({

progressNumerical: params.progressNumerical,

accentColor: params.accentColor

})

});

// Generate the status card.

(vars.statusWidth, vars.statusCard) =

SVGElements.card({ cardType: SVGElements.CardType.STATUS, content: params.status });

// Generate the deposit amount card.

(vars.amountWidth, vars.amountCard) =

SVGElements.card({ cardType: SVGElements.CardType.AMOUNT, content: params.amount });

// Generate the duration card.

(vars.durationWidth, vars.durationCard) =

SVGElements.card({ cardType: SVGElements.CardType.DURATION, content: params.duration });

unchecked {

// Calculate the width of the row containing the cards and the margins between them.

vars.cardsWidth =

vars.amountWidth + vars.durationWidth + vars.progressWidth + vars.statusWidth + CARD_MARGIN * 3;

// Calculate the positions on the X axis based on the following layout:

// ___________________________ SVG Width (1000px) ___________________________

// | | | | | | | | | |

// | <-> | Progress | 16px | Status | 16px | Amount | 16px | Duration | <-> |

vars.progressXPosition = (1000 - vars.cardsWidth) / 2;

vars.statusXPosition = vars.progressXPosition + vars.progressWidth + CARD_MARGIN;

vars.amountXPosition = vars.statusXPosition + vars.statusWidth + CARD_MARGIN;

vars.durationXPosition = vars.amountXPosition + vars.amountWidth + CARD_MARGIN;

}

// Concatenate all cards.

vars.cards = string.concat(vars.progressCard, vars.statusCard, vars.amountCard, vars.durationCard);

return string.concat(

'<svg xmlns="http://www.w3.org/2000/svg" width="1000" height="1000" viewBox="0 0 1000 1000">',

SVGElements.BACKGROUND,

generateDefs(params.accentColor, params.status, vars.cards),

generateFloatingText(params.sablierAddress, params.sablierModel, params.assetAddress, params.assetSymbol),

generateHrefs(vars.progressXPosition, vars.statusXPosition, vars.amountXPosition, vars.durationXPosition),

"</svg>"

);

}

function generateDefs(

string memory accentColor,

string memory status,

string memory cards

)

internal

pure

returns (string memory)

{

return string.concat(

"<defs>",

SVGElements.GLOW,

SVGElements.NOISE,

SVGElements.LOGO,

SVGElements.FLOATING_TEXT,

SVGElements.gradients(accentColor),

SVGElements.hourglass(status),

cards,

"</defs>"

);

}

function generateFloatingText(

string memory sablierAddress,

string memory sablierModel,

string memory assetAddress,

string memory assetSymbol

)

internal

pure

returns (string memory)

{

return string.concat(

'<text text-rendering="optimizeSpeed">',

SVGElements.floatingText({

offset: "-100%",

text: string.concat(sablierAddress, unicode" • ", "Sablier V2 ", sablierModel)

}),

SVGElements.floatingText({

offset: "0%",

text: string.concat(sablierAddress, unicode" • ", "Sablier V2 ", sablierModel)

}),

SVGElements.floatingText({ offset: "-50%", text: string.concat(assetAddress, unicode" • ", assetSymbol) }),

SVGElements.floatingText({ offset: "50%", text: string.concat(assetAddress, unicode" • ", assetSymbol) }),

"</text>"

);

}

function generateHrefs(

uint256 progressXPosition,

uint256 statusXPosition,

uint256 amountXPosition,

uint256 durationXPosition

)

internal

pure

returns (string memory)

{

return string.concat(

'<use href="#Glow" fill-opacity=".9"/>',

'<use href="#Glow" x="1000" y="1000" fill-opacity=".9"/>',

'<use href="#Logo" x="170" y="170" transform="scale(.6)"/>'

'<use href="#Hourglass" x="150" y="90" transform="rotate(10)" transform-origin="500 500"/>',

'<use href="#Progress" x="',

progressXPosition.toString(),

'" y="790"/>',

'<use href="#Status" x="',

statusXPosition.toString(),

'" y="790"/>',

'<use href="#Amount" x="',

amountXPosition.toString(),

'" y="790"/>',

'<use href="#Duration" x="',

durationXPosition.toString(),

'" y="790"/>'

);

}

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/libraries/NFTSVG.sol#L1:1

NC014 - Function declarations should have NatSpec descriptions:

Function declarations should be preceded by a NatSpec comment.

File: v2-core/src/libraries/NFTSVG.sol

44 function generateSVG(SVGParams memory params) internal pure returns (string memory) {

98 function generateDefs(

120 function generateFloatingText(

146 function generateHrefs(

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/libraries/NFTSVG.sol#L0:0

File: v2-core/src/libraries/SVGElements.sol

74 function card(CardType cardType, string memory content) internal pure returns (uint256, string memory) {

78 function card(

128 function floatingText(string memory offset, string memory text) internal pure returns (string memory) {

139 function gradients(string memory accentColor) internal pure returns (string memory) {

185 function hourglass(string memory status) internal pure returns (string memory) {

198 function progressCircle(

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/libraries/SVGElements.sol#L0:0

NC015 - Contract declarations should have `@notice` tags:

@notice is used to explain to end users what the contract does, and the compiler interprets /// or /** comments as this tag if one wasn't explicitly provided.

File: v2-core/src/libraries/NFTSVG.sol

9 library NFTSVG {

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/libraries/NFTSVG.sol#L0:0

File: v2-core/src/libraries/SVGElements.sol

8 library SVGElements {

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/libraries/SVGElements.sol#L0:0

File: v2-periphery/src/types/DataTypes.sol

9 library BatchLockup {

91 library MerkleLockup {

113 library MerkleLT {

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/types/DataTypes.sol#L0:0

NC016 - Error declarations should have NatSpec descriptions:

File: v2-periphery/src/libraries/Errors.sol

11 error SablierV2BatchLockup_BatchSizeZero();

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-periphery/src/libraries/Errors.sol#L0:0

NC017 - Contract does not follow the Solidity style guide's suggested layout ordering:

The style guide says that, within a contract, the ordering should be 1) Type declarations, 2) State variables, 3) Events, 4) Modifiers, and 5) Functions, but the contract(s) below do not follow this ordering.

File: v2-core/src/abstracts/NoDelegateCall.sol

18 modifier noDelegateCall() {

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/abstracts/NoDelegateCall.sol#L0:0

File: v2-core/src/abstracts/SablierV2Lockup.sol

65 modifier notNull(uint256 streamId) {

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/abstracts/SablierV2Lockup.sol#L0:0

NC018 - Non-external/public variable names should begin with an underscore:

According to the Solidity Style Guide, non-external/public variable names should begin with an underscore

File: v2-core/src/abstracts/NoDelegateCall.sol

10 address private immutable ORIGINAL;

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/abstracts/NoDelegateCall.sol#L0:0

File: v2-periphery/src/abstracts/SablierV2MerkleLockup.sol

40 bytes32 internal immutable NAME;

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-periphery/src/abstracts/SablierV2MerkleLockup.sol#L0:0

NC019 - Non-library/interface files should use fixed compiler versions, not floating ones:

Using a floating compiler version like ^0.8.16 or >=0.8.16 can lead to unexpected behavior if the compiler version used differs from the one intended. It's recommended to specify a fixed compiler version for non-library/interface files.

Click to show 8 findings

File: v2-core/src/SablierV2LockupDynamic.sol

2 pragma solidity >=0.8.22;

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/SablierV2LockupDynamic.sol#L0:0

File: v2-core/src/SablierV2LockupLinear.sol

2 pragma solidity >=0.8.22;

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/SablierV2LockupLinear.sol#L0:0

File: v2-core/src/SablierV2LockupTranched.sol

2 pragma solidity >=0.8.22;

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/SablierV2LockupTranched.sol#L0:0

File: v2-core/src/SablierV2NFTDescriptor.sol

3 pragma solidity >=0.8.22;

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/SablierV2NFTDescriptor.sol#L0:0

File: v2-periphery/src/SablierV2BatchLockup.sol

2 pragma solidity >=0.8.22;

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-periphery/src/SablierV2BatchLockup.sol#L0:0

File: v2-periphery/src/SablierV2MerkleLL.sol

2 pragma solidity >=0.8.22;

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-periphery/src/SablierV2MerkleLL.sol#L0:0

File: v2-periphery/src/SablierV2MerkleLT.sol

2 pragma solidity >=0.8.22;

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-periphery/src/SablierV2MerkleLT.sol#L0:0

File: v2-periphery/src/SablierV2MerkleLockupFactory.sol

2 pragma solidity >=0.8.22;

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-periphery/src/SablierV2MerkleLockupFactory.sol#L0:0

NC020 - Contracts should have full test coverage:

While 100% code coverage does not guarantee that there are no bugs, it often will catch easy-to-find bugs, and will ensure that there are fewer regressions when the code invariably has to be modified. Furthermore, in order to get full coverage, code authors will often have to re-organize their code so that it is more modular, so that each component can be tested separately, which reduces interdependencies between modules and layers, and makes for code that is easier to reason about and audit.Summary

Vulnerability Details

Impact

Tools Used

Recommendations

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago

Submission Judgement Published

Invalidated

Reason: Non-acceptable severity

Assigned finding tags:

Info/Gas/Invalid as per Docs

https://docs.codehawks.com/hawks-auditors/how-to-determine-a-finding-validity

Prize pool breakdown

Total prize

53,440 USDC

nSLOC

2,655

20 USDC / LOC

High Medium

45,000 USDC

Low

3,100 USDC

Judges

5,340 USDC

Live

The contest is live. Earn rewards by submitting a finding.

Judging

Submissions are being carefully reviewed by our judges.

View all submissions

Appeals

This is your time to appeal against judgements on your submissions.

Appeals Review

Appeals are being carefully reviewed by our judges.

Rewards Distribution

The contest is complete and the rewards are being distributed.

View results

Support

FAQs

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

Sablier

Non Critical issues (11-20) of 73

NC011 - Consider using delete rather than assigning false/zero to clear values:

NC012 - Lines are too long:

NC013 - File is missing NatSpec comments:

NC014 - Function declarations should have NatSpec descriptions:

NC015 - Contract declarations should have @notice tags:

NC016 - Error declarations should have NatSpec descriptions:

NC017 - Contract does not follow the Solidity style guide's suggested layout ordering:

NC018 - Non-external/public variable names should begin with an underscore:

NC019 - Non-library/interface files should use fixed compiler versions, not floating ones:

NC020 - Contracts should have full test coverage:

Vulnerability Details

Impact

Tools Used

Recommendations

Lead Judging Commences

Info/Gas/Invalid as per Docs

Prize pool breakdown

Live

Judging

Appeals

Appeals Review

Rewards Distribution

FAQs

NC015 - Contract declarations should have `@notice` tags: