Submission Details
Severity:
Non Critical issues (40-46) of 73
NC040 - It is standard for all external and public functions to be override from an interface:
This is to ensure the whole API is extracted in an interface
Click to show 8 findings
File: v2-core/src/SablierV2LockupDynamic.sol
80 function getSegments(uint256 streamId)
81 external
82 view
83 override
84 notNull(streamId)
85 returns (LockupDynamic.Segment[] memory segments)
86 {
87 segments = _segments[streamId];
88 }
91 function getStream(uint256 streamId)
92 external
93 view
94 override
95 notNull(streamId)
96 returns (LockupDynamic.StreamLD memory stream)
97 {
98 // Retrieve the Lockup stream from storage.
99 Lockup.Stream memory lockupStream = _streams[streamId];
100
101 // Settled streams cannot be canceled.
102 if (_statusOf(streamId) == Lockup.Status.SETTLED) {
103 lockupStream.isCancelable = false;
104 }
105
106 stream = LockupDynamic.StreamLD({
107 amounts: lockupStream.amounts,
108 asset: lockupStream.asset,
109 endTime: lockupStream.endTime,
110 isCancelable: lockupStream.isCancelable,
111 isDepleted: lockupStream.isDepleted,
112 isStream: lockupStream.isStream,
113 isTransferable: lockupStream.isTransferable,
114 recipient: _ownerOf(streamId),
115 segments: _segments[streamId],
116 sender: lockupStream.sender,
117 startTime: lockupStream.startTime,
118 wasCanceled: lockupStream.wasCanceled
119 });
120 }
123 function getTimestamps(uint256 streamId)
124 external
125 view
126 override
127 notNull(streamId)
128 returns (LockupDynamic.Timestamps memory timestamps)
129 {
130 timestamps = LockupDynamic.Timestamps({ start: _streams[streamId].startTime, end: _streams[streamId].endTime });
131 }
138 function createWithDurations(LockupDynamic.CreateWithDurations calldata params)
139 external
140 override
141 noDelegateCall
142 returns (uint256 streamId)
143 {
144 // Generate the canonical segments.
145 LockupDynamic.Segment[] memory segments = Helpers.calculateSegmentTimestamps(params.segments);
146
147 // Checks, Effects and Interactions: create the stream.
148 streamId = _create(
149 LockupDynamic.CreateWithTimestamps({
150 sender: params.sender,
151 recipient: params.recipient,
152 totalAmount: params.totalAmount,
153 asset: params.asset,
154 cancelable: params.cancelable,
155 transferable: params.transferable,
156 startTime: uint40(block.timestamp),
157 segments: segments,
158 broker: params.broker
159 })
160 );
161 }
164 function createWithTimestamps(LockupDynamic.CreateWithTimestamps calldata params)
165 external
166 override
167 noDelegateCall
168 returns (uint256 streamId)
169 {
170 // Checks, Effects and Interactions: create the stream.
171 streamId = _create(params);
172 }
https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/SablierV2LockupDynamic.sol#L0:0
File: v2-core/src/SablierV2LockupLinear.sol
71 function getCliffTime(uint256 streamId) external view override notNull(streamId) returns (uint40 cliffTime) {
72 cliffTime = _cliffs[streamId];
73 }
76 function getStream(uint256 streamId)
77 external
78 view
79 override
80 notNull(streamId)
81 returns (LockupLinear.StreamLL memory stream)
82 {
83 // Retrieve the Lockup stream from storage.
84 Lockup.Stream memory lockupStream = _streams[streamId];
85
86 // Settled streams cannot be canceled.
87 if (_statusOf(streamId) == Lockup.Status.SETTLED) {
88 lockupStream.isCancelable = false;
89 }
90
91 stream = LockupLinear.StreamLL({
92 amounts: lockupStream.amounts,
93 asset: lockupStream.asset,
94 cliffTime: _cliffs[streamId],
95 endTime: lockupStream.endTime,
96 isCancelable: lockupStream.isCancelable,
97 isTransferable: lockupStream.isTransferable,
98 isDepleted: lockupStream.isDepleted,
99 isStream: lockupStream.isStream,
100 recipient: _ownerOf(streamId),
101 sender: lockupStream.sender,
102 startTime: lockupStream.startTime,
103 wasCanceled: lockupStream.wasCanceled
104 });
105 }
108 function getTimestamps(uint256 streamId)
109 external
110 view
111 override
112 notNull(streamId)
113 returns (LockupLinear.Timestamps memory timestamps)
114 {
115 timestamps = LockupLinear.Timestamps({
116 start: _streams[streamId].startTime,
117 cliff: _cliffs[streamId],
118 end: _streams[streamId].endTime
119 });
120 }
127 function createWithDurations(LockupLinear.CreateWithDurations calldata params)
128 external
129 override
130 noDelegateCall
131 returns (uint256 streamId)
132 {
133 // Set the current block timestamp as the stream's start time.
134 LockupLinear.Timestamps memory timestamps;
135 timestamps.start = uint40(block.timestamp);
136
137 // Calculate the cliff time and the end time. It is safe to use unchecked arithmetic because {_create} will
138 // nonetheless check that the end time is greater than the cliff time, and also that the cliff time, if set,
139 // is greater than or equal to the start time.
140 unchecked {
141 if (params.durations.cliff > 0) {
142 timestamps.cliff = timestamps.start + params.durations.cliff;
143 }
144 timestamps.end = timestamps.start + params.durations.total;
145 }
146
147 // Checks, Effects and Interactions: create the stream.
148 streamId = _create(
149 LockupLinear.CreateWithTimestamps({
150 sender: params.sender,
151 recipient: params.recipient,
152 totalAmount: params.totalAmount,
153 asset: params.asset,
154 cancelable: params.cancelable,
155 transferable: params.transferable,
156 timestamps: timestamps,
157 broker: params.broker
158 })
159 );
160 }
163 function createWithTimestamps(LockupLinear.CreateWithTimestamps calldata params)
164 external
165 override
166 noDelegateCall
167 returns (uint256 streamId)
168 {
169 // Checks, Effects and Interactions: create the stream.
170 streamId = _create(params);
171 }
https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/SablierV2LockupLinear.sol#L0:0
File: v2-core/src/SablierV2LockupTranched.sol
75 function getStream(uint256 streamId)
76 external
77 view
78 override
79 notNull(streamId)
80 returns (LockupTranched.StreamLT memory stream)
81 {
82 // Retrieve the Lockup stream from storage.
83 Lockup.Stream memory lockupStream = _streams[streamId];
84
85 // Settled streams cannot be canceled.
86 if (_statusOf(streamId) == Lockup.Status.SETTLED) {
87 lockupStream.isCancelable = false;
88 }
89
90 stream = LockupTranched.StreamLT({
91 amounts: lockupStream.amounts,
92 asset: lockupStream.asset,
93 endTime: lockupStream.endTime,
94 isCancelable: lockupStream.isCancelable,
95 isDepleted: lockupStream.isDepleted,
96 isStream: lockupStream.isStream,
97 isTransferable: lockupStream.isTransferable,
98 recipient: _ownerOf(streamId),
99 sender: lockupStream.sender,
100 startTime: lockupStream.startTime,
101 tranches: _tranches[streamId],
102 wasCanceled: lockupStream.wasCanceled
103 });
104 }
107 function getTimestamps(uint256 streamId)
108 external
109 view
110 override
111 notNull(streamId)
112 returns (LockupTranched.Timestamps memory timestamps)
113 {
114 timestamps = LockupTranched.Timestamps({ start: _streams[streamId].startTime, end: _streams[streamId].endTime });
115 }
118 function getTranches(uint256 streamId)
119 external
120 view
121 override
122 notNull(streamId)
123 returns (LockupTranched.Tranche[] memory tranches)
124 {
125 tranches = _tranches[streamId];
126 }
133 function createWithDurations(LockupTranched.CreateWithDurations calldata params)
134 external
135 override
136 noDelegateCall
137 returns (uint256 streamId)
138 {
139 // Generate the canonical tranches.
140 LockupTranched.Tranche[] memory tranches = Helpers.calculateTrancheTimestamps(params.tranches);
141
142 // Checks, Effects and Interactions: create the stream.
143 streamId = _create(
144 LockupTranched.CreateWithTimestamps({
145 sender: params.sender,
146 recipient: params.recipient,
147 totalAmount: params.totalAmount,
148 asset: params.asset,
149 cancelable: params.cancelable,
150 transferable: params.transferable,
151 startTime: uint40(block.timestamp),
152 tranches: tranches,
153 broker: params.broker
154 })
155 );
156 }
159 function createWithTimestamps(LockupTranched.CreateWithTimestamps calldata params)
160 external
161 override
162 noDelegateCall
163 returns (uint256 streamId)
164 {
165 // Checks, Effects and Interactions: create the stream.
166 streamId = _create(params);
167 }
https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/SablierV2LockupTranched.sol#L0:0
File: v2-core/src/SablierV2NFTDescriptor.sol
47 function tokenURI(IERC721Metadata sablier, uint256 streamId) external view override returns (string memory uri) {
48 TokenURIVars memory vars;
49
50 // Load the contracts.
51 vars.sablier = ISablierV2Lockup(address(sablier));
52 vars.sablierModel = mapSymbol(sablier);
53 vars.sablierStringified = address(sablier).toHexString();
54 vars.asset = address(vars.sablier.getAsset(streamId));
55 vars.assetSymbol = safeAssetSymbol(vars.asset);
56 vars.depositedAmount = vars.sablier.getDepositedAmount(streamId);
57
58 // Load the stream's data.
59 vars.status = stringifyStatus(vars.sablier.statusOf(streamId));
60 vars.streamedPercentage = calculateStreamedPercentage({
61 streamedAmount: vars.sablier.streamedAmountOf(streamId),
62 depositedAmount: vars.depositedAmount
63 });
64
65 // Generate the SVG.
66 vars.svg = NFTSVG.generateSVG(
67 NFTSVG.SVGParams({
68 accentColor: generateAccentColor(address(sablier), streamId),
69 amount: abbreviateAmount({ amount: vars.depositedAmount, decimals: safeAssetDecimals(vars.asset) }),
70 assetAddress: vars.asset.toHexString(),
71 assetSymbol: vars.assetSymbol,
72 duration: calculateDurationInDays({
73 startTime: vars.sablier.getStartTime(streamId),
74 endTime: vars.sablier.getEndTime(streamId)
75 }),
76 sablierAddress: vars.sablierStringified,
77 progress: stringifyPercentage(vars.streamedPercentage),
78 progressNumerical: vars.streamedPercentage,
79 status: vars.status,
80 sablierModel: vars.sablierModel
81 })
82 );
83
84 // Performs a low-level call to handle older deployments that miss the `isTransferable` function.
85 (vars.success, vars.returnData) =
86 address(vars.sablier).staticcall(abi.encodeCall(ISablierV2Lockup.isTransferable, (streamId)));
87
88 // When the call has failed, the stream NFT is assumed to be transferable.
89 vars.isTransferable = vars.success ? abi.decode(vars.returnData, (bool)) : true;
90
91 // Generate the JSON metadata.
92 vars.json = string.concat(
93 '{"attributes":',
94 generateAttributes({
95 assetSymbol: vars.assetSymbol,
96 sender: vars.sablier.getSender(streamId).toHexString(),
97 status: vars.status
98 }),
99 ',"description":"',
100 generateDescription({
101 sablierModel: vars.sablierModel,
102 assetSymbol: vars.assetSymbol,
103 sablierStringified: vars.sablierStringified,
104 assetAddress: vars.asset.toHexString(),
105 streamId: streamId.toString(),
106 isTransferable: vars.isTransferable
107 }),
108 '","external_url":"https://sablier.com","name":"',
109 generateName({ sablierModel: vars.sablierModel, streamId: streamId.toString() }),
110 '","image":"data:image/svg+xml;base64,',
111 Base64.encode(bytes(vars.svg)),
112 '"}'
113 );
114
115 // Encode the JSON metadata in Base64.
116 uri = string.concat("data:application/json;base64,", Base64.encode(bytes(vars.json)));
117 }
https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/SablierV2NFTDescriptor.sol#L0:0
File: v2-periphery/src/SablierV2BatchLockup.sol
25 function createWithDurationsLD(
26 ISablierV2LockupDynamic lockupDynamic,
27 IERC20 asset,
28 BatchLockup.CreateWithDurationsLD[] calldata batch
29 )
30 external
31 override
32 returns (uint256[] memory streamIds)
33 {
34 // Check that the batch size is not zero.
35 uint256 batchSize = batch.length;
36 if (batchSize == 0) {
37 revert Errors.SablierV2BatchLockup_BatchSizeZero();
38 }
39
40 // Calculate the sum of all of stream amounts. It is safe to use unchecked addition because one of the create
41 // transactions will revert if there is overflow.
42 uint256 i;
43 uint256 transferAmount;
44 for (i = 0; i < batchSize; ++i) {
45 unchecked {
46 transferAmount += batch[i].totalAmount;
47 }
48 }
49
50 // Perform the ERC-20 transfer and approve {SablierV2LockupDynamic} to spend the amount of assets.
51 _handleTransfer(address(lockupDynamic), asset, transferAmount);
52
53 // Create a stream for each element in the parameter array.
54 streamIds = new uint256[](batchSize);
55 for (i = 0; i < batchSize; ++i) {
56 // Create the stream.
57 streamIds[i] = lockupDynamic.createWithDurations(
58 LockupDynamic.CreateWithDurations({
59 sender: batch[i].sender,
60 recipient: batch[i].recipient,
61 totalAmount: batch[i].totalAmount,
62 asset: asset,
63 cancelable: batch[i].cancelable,
64 transferable: batch[i].transferable,
65 segments: batch[i].segments,
66 broker: batch[i].broker
67 })
68 );
69 }
70 }
73 function createWithTimestampsLD(
74 ISablierV2LockupDynamic lockupDynamic,
75 IERC20 asset,
76 BatchLockup.CreateWithTimestampsLD[] calldata batch
77 )
78 external
79 override
80 returns (uint256[] memory streamIds)
81 {
82 // Check that the batch size is not zero.
83 uint256 batchSize = batch.length;
84 if (batchSize == 0) {
85 revert Errors.SablierV2BatchLockup_BatchSizeZero();
86 }
87
88 // Calculate the sum of all of stream amounts. It is safe to use unchecked addition because one of the create
89 // transactions will revert if there is overflow.
90 uint256 i;
91 uint256 transferAmount;
92 for (i = 0; i < batchSize; ++i) {
93 unchecked {
94 transferAmount += batch[i].totalAmount;
95 }
96 }
97
98 // Perform the ERC-20 transfer and approve {SablierV2LockupDynamic} to spend the amount of assets.
99 _handleTransfer(address(lockupDynamic), asset, transferAmount);
100
101 // Create a stream for each element in the parameter array.
102 streamIds = new uint256[](batchSize);
103 for (i = 0; i < batchSize; ++i) {
104 // Create the stream.
105 streamIds[i] = lockupDynamic.createWithTimestamps(
106 LockupDynamic.CreateWithTimestamps({
107 sender: batch[i].sender,
108 recipient: batch[i].recipient,
109 totalAmount: batch[i].totalAmount,
110 asset: asset,
111 cancelable: batch[i].cancelable,
112 transferable: batch[i].transferable,
113 startTime: batch[i].startTime,
114 segments: batch[i].segments,
115 broker: batch[i].broker
116 })
117 );
118 }
119 }
126 function createWithDurationsLL(
127 ISablierV2LockupLinear lockupLinear,
128 IERC20 asset,
129 BatchLockup.CreateWithDurationsLL[] calldata batch
130 )
131 external
132 override
133 returns (uint256[] memory streamIds)
134 {
135 // Check that the batch size is not zero.
136 uint256 batchSize = batch.length;
137 if (batchSize == 0) {
138 revert Errors.SablierV2BatchLockup_BatchSizeZero();
139 }
140
141 // Calculate the sum of all of stream amounts. It is safe to use unchecked addition because one of the create
142 // transactions will revert if there is overflow.
143 uint256 i;
144 uint256 transferAmount;
145 for (i = 0; i < batchSize; ++i) {
146 unchecked {
147 transferAmount += batch[i].totalAmount;
148 }
149 }
150
151 // Perform the ERC-20 transfer and approve {SablierV2LockupLinear} to spend the amount of assets.
152 _handleTransfer(address(lockupLinear), asset, transferAmount);
153
154 // Create a stream for each element in the parameter array.
155 streamIds = new uint256[](batchSize);
156 for (i = 0; i < batchSize; ++i) {
157 // Create the stream.
158 streamIds[i] = lockupLinear.createWithDurations(
159 LockupLinear.CreateWithDurations({
160 sender: batch[i].sender,
161 recipient: batch[i].recipient,
162 totalAmount: batch[i].totalAmount,
163 asset: asset,
164 cancelable: batch[i].cancelable,
165 transferable: batch[i].transferable,
166 durations: batch[i].durations,
167 broker: batch[i].broker
168 })
169 );
170 }
171 }
174 function createWithTimestampsLL(
175 ISablierV2LockupLinear lockupLinear,
176 IERC20 asset,
177 BatchLockup.CreateWithTimestampsLL[] calldata batch
178 )
179 external
180 override
181 returns (uint256[] memory streamIds)
182 {
183 // Check that the batch is not empty.
184 uint256 batchSize = batch.length;
185 if (batchSize == 0) {
186 revert Errors.SablierV2BatchLockup_BatchSizeZero();
187 }
188
189 // Calculate the sum of all of stream amounts. It is safe to use unchecked addition because one of the create
190 // transactions will revert if there is overflow.
191 uint256 i;
192 uint256 transferAmount;
193 for (i = 0; i < batchSize; ++i) {
194 unchecked {
195 transferAmount += batch[i].totalAmount;
196 }
197 }
198
199 // Perform the ERC-20 transfer and approve {SablierV2LockupLinear} to spend the amount of assets.
200 _handleTransfer(address(lockupLinear), asset, transferAmount);
201
202 // Create a stream for each element in the parameter array.
203 streamIds = new uint256[](batchSize);
204 for (i = 0; i < batchSize; ++i) {
205 // Create the stream.
206 streamIds[i] = lockupLinear.createWithTimestamps(
207 LockupLinear.CreateWithTimestamps({
208 sender: batch[i].sender,
209 recipient: batch[i].recipient,
210 totalAmount: batch[i].totalAmount,
211 asset: asset,
212 cancelable: batch[i].cancelable,
213 transferable: batch[i].transferable,
214 timestamps: batch[i].timestamps,
215 broker: batch[i].broker
216 })
217 );
218 }
219 }
226 function createWithDurationsLT(
227 ISablierV2LockupTranched lockupTranched,
228 IERC20 asset,
229 BatchLockup.CreateWithDurationsLT[] calldata batch
230 )
231 external
232 override
233 returns (uint256[] memory streamIds)
234 {
235 // Check that the batch size is not zero.
236 uint256 batchSize = batch.length;
237 if (batchSize == 0) {
238 revert Errors.SablierV2BatchLockup_BatchSizeZero();
239 }
240
241 // Calculate the sum of all of stream amounts. It is safe to use unchecked addition because one of the create
242 // transactions will revert if there is overflow.
243 uint256 i;
244 uint256 transferAmount;
245 for (i = 0; i < batchSize; ++i) {
246 unchecked {
247 transferAmount += batch[i].totalAmount;
248 }
249 }
250
251 // Perform the ERC-20 transfer and approve {SablierV2LockupTranched} to spend the amount of assets.
252 _handleTransfer(address(lockupTranched), asset, transferAmount);
253
254 // Create a stream for each element in the parameter array.
255 streamIds = new uint256[](batchSize);
256 for (i = 0; i < batchSize; ++i) {
257 // Create the stream.
258 streamIds[i] = lockupTranched.createWithDurations(
259 LockupTranched.CreateWithDurations({
260 sender: batch[i].sender,
261 recipient: batch[i].recipient,
262 totalAmount: batch[i].totalAmount,
263 asset: asset,
264 cancelable: batch[i].cancelable,
265 transferable: batch[i].transferable,
266 tranches: batch[i].tranches,
267 broker: batch[i].broker
268 })
269 );
270 }
271 }
274 function createWithTimestampsLT(
275 ISablierV2LockupTranched lockupTranched,
276 IERC20 asset,
277 BatchLockup.CreateWithTimestampsLT[] calldata batch
278 )
279 external
280 override
281 returns (uint256[] memory streamIds)
282 {
283 // Check that the batch size is not zero.
284 uint256 batchSize = batch.length;
285 if (batchSize == 0) {
286 revert Errors.SablierV2BatchLockup_BatchSizeZero();
287 }
288
289 // Calculate the sum of all of stream amounts. It is safe to use unchecked addition because one of the create
290 // transactions will revert if there is overflow.
291 uint256 i;
292 uint256 transferAmount;
293 for (i = 0; i < batchSize; ++i) {
294 unchecked {
295 transferAmount += batch[i].totalAmount;
296 }
297 }
298
299 // Perform the ERC-20 transfer and approve {SablierV2LockupTranched} to spend the amount of assets.
300 _handleTransfer(address(lockupTranched), asset, transferAmount);
301
302 // Create a stream for each element in the parameter array.
303 streamIds = new uint256[](batchSize);
304 for (i = 0; i < batchSize; ++i) {
305 // Create the stream.
306 streamIds[i] = lockupTranched.createWithTimestamps(
307 LockupTranched.CreateWithTimestamps({
308 sender: batch[i].sender,
309 recipient: batch[i].recipient,
310 totalAmount: batch[i].totalAmount,
311 asset: asset,
312 cancelable: batch[i].cancelable,
313 transferable: batch[i].transferable,
314 startTime: batch[i].startTime,
315 tranches: batch[i].tranches,
316 broker: batch[i].broker
317 })
318 );
319 }
320 }
https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-periphery/src/SablierV2BatchLockup.sol#L0:0
File: v2-periphery/src/SablierV2MerkleLL.sol
59 function claim(
60 uint256 index,
61 address recipient,
62 uint128 amount,
63 bytes32[] calldata merkleProof
64 )
65 external
66 override
67 returns (uint256 streamId)
68 {
69 // Generate the Merkle tree leaf by hashing the corresponding parameters. Hashing twice prevents second
70 // preimage attacks.
71 bytes32 leaf = keccak256(bytes.concat(keccak256(abi.encode(index, recipient, amount))));
72
73 // Check: validate the function.
74 _checkClaim(index, leaf, merkleProof);
75
76 // Effect: mark the index as claimed.
77 _claimedBitMap.set(index);
78
79 // Interaction: create the stream via {SablierV2LockupLinear}.
80 streamId = LOCKUP_LINEAR.createWithDurations(
81 LockupLinear.CreateWithDurations({
82 sender: admin,
83 recipient: recipient,
84 totalAmount: amount,
85 asset: ASSET,
86 cancelable: CANCELABLE,
87 transferable: TRANSFERABLE,
88 durations: streamDurations,
89 broker: Broker({ account: address(0), fee: ud(0) })
90 })
91 );
92
93 // Log the claim.
94 emit Claim(index, recipient, amount, streamId);
95 }
https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-periphery/src/SablierV2MerkleLL.sol#L0:0
File: v2-periphery/src/SablierV2MerkleLT.sol
65 function getTranchesWithPercentages() external view override returns (MerkleLT.TrancheWithPercentage[] memory) {
66 return _tranchesWithPercentages;
67 }
74 function claim(
75 uint256 index,
76 address recipient,
77 uint128 amount,
78 bytes32[] calldata merkleProof
79 )
80 external
81 override
82 returns (uint256 streamId)
83 {
84 // Generate the Merkle tree leaf by hashing the corresponding parameters. Hashing twice prevents second
85 // preimage attacks.
86 bytes32 leaf = keccak256(bytes.concat(keccak256(abi.encode(index, recipient, amount))));
87
88 // Check: validate the function.
89 _checkClaim(index, leaf, merkleProof);
90
91 // Calculate the tranches based on the unlock percentages.
92 LockupTranched.TrancheWithDuration[] memory tranches = _calculateTranches(amount);
93
94 // Effect: mark the index as claimed.
95 _claimedBitMap.set(index);
96
97 // Interaction: create the stream via {SablierV2LockupTranched}.
98 streamId = LOCKUP_TRANCHED.createWithDurations(
99 LockupTranched.CreateWithDurations({
100 sender: admin,
101 recipient: recipient,
102 totalAmount: amount,
103 asset: ASSET,
104 cancelable: CANCELABLE,
105 transferable: TRANSFERABLE,
106 tranches: tranches,
107 broker: Broker({ account: address(0), fee: ZERO })
108 })
109 );
110
111 // Log the claim.
112 emit Claim(index, recipient, amount, streamId);
113 }
https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-periphery/src/SablierV2MerkleLT.sol#L0:0
File: v2-periphery/src/SablierV2MerkleLockupFactory.sol
25 function createMerkleLL(
26 MerkleLockup.ConstructorParams memory baseParams,
27 ISablierV2LockupLinear lockupLinear,
28 LockupLinear.Durations memory streamDurations,
29 uint256 aggregateAmount,
30 uint256 recipientCount
31 )
32 external
33 returns (ISablierV2MerkleLL merkleLL)
34 {
35 // Deploy the MerkleLockup contract with CREATE.
36 merkleLL = new SablierV2MerkleLL(baseParams, lockupLinear, streamDurations);
37
38 // Log the creation of the MerkleLockup contract, including some metadata that is not stored on-chain.
39 emit CreateMerkleLL(merkleLL, baseParams, lockupLinear, streamDurations, aggregateAmount, recipientCount);
40 }
43 function createMerkleLT(
44 MerkleLockup.ConstructorParams memory baseParams,
45 ISablierV2LockupTranched lockupTranched,
46 MerkleLT.TrancheWithPercentage[] memory tranchesWithPercentages,
47 uint256 aggregateAmount,
48 uint256 recipientCount
49 )
50 external
51 returns (ISablierV2MerkleLT merkleLT)
52 {
53 // Calculate the sum of percentages and durations across all tranches.
54 uint64 totalPercentage;
55 uint256 totalDuration;
56 for (uint256 i = 0; i < tranchesWithPercentages.length; ++i) {
57 uint64 percentage = tranchesWithPercentages[i].unlockPercentage.unwrap();
58 totalPercentage = totalPercentage + percentage;
59 unchecked {
60 // Safe to use `unchecked` because its only used in the event.
61 totalDuration += tranchesWithPercentages[i].duration;
62 }
63 }
64
65 // Check: the sum of percentages equals 100%.
66 if (totalPercentage != uUNIT) {
67 revert Errors.SablierV2MerkleLockupFactory_TotalPercentageNotOneHundred(totalPercentage);
68 }
69
70 // Deploy the MerkleLockup contract with CREATE.
71 merkleLT = new SablierV2MerkleLT(baseParams, lockupTranched, tranchesWithPercentages);
72
73 // Log the creation of the MerkleLockup contract, including some metadata that is not stored on-chain.
74 emit CreateMerkleLT(
75 merkleLT,
76 baseParams,
77 lockupTranched,
78 tranchesWithPercentages,
79 totalDuration,
80 aggregateAmount,
81 recipientCount
82 );
83 }
https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-periphery/src/SablierV2MerkleLockupFactory.sol#L0:0
NC041 - Consider adding formal verification proofs:
Consider using formal verification to mathematically prove that your code does what is intended, and does not have any edge cases with unexpected behavior. The solidity compiler itself has this functionality built in based off of SMTChecker.
File: Various Files
None
NC042 - Large multiples of ten should use scientific notation (e.g. 1e6) rather than decimal literals (e.g. 1000000), for readability:
Using scientific notation for large multiples of ten improves code readability. Instead of writing large decimal literals, consider using scientific notation.
File: v2-core/src/SablierV2NFTDescriptor.sol
200 return streamedAmount * 10_000 / depositedAmount;
https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/SablierV2NFTDescriptor.sol#L0:0
File: v2-core/src/libraries/SVGElements.sol
217 (10_000 - progressNumerical).toString(),
https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/libraries/SVGElements.sol#L0:0
NC043 - Polymorphic functions make security audits more time-consuming and error-prone:
The instances below point to one of two functions with the same name. Consider naming each function differently, in order to make code navigation and analysis easier.
File: v2-core/src/libraries/SVGElements.sol
78 function card(
https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/libraries/SVGElements.sol#L0:0
NC044 - Missing timelock for critical parameter change:
Timelocks prevent users from being surprised by changes.
File: v2-core/src/abstracts/SablierV2Lockup.sol
318 nftDescriptor = newNFTDescriptor;
https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/abstracts/SablierV2Lockup.sol#L0:0
NC045 - Consider splitting long calculations:
The longer a string of operations is, the harder it is to understand it. Consider splitting the full calculation into more steps, with more descriptive temporary variable names, and add extensive comments.
File: v2-core/src/abstracts/SablierV2Lockup.sol
151 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#L0:0
File: v2-core/src/libraries/NFTSVG.sol
71 vars.cardsWidth =
72 vars.amountWidth + vars.durationWidth + vars.progressWidth + vars.statusWidth + CARD_MARGIN * 3;
https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/libraries/NFTSVG.sol#L0:0
NC046 - High cyclomatic complexity:
Consider breaking down these blocks into more manageable units, by splitting things into utility functions, by reducing nesting, and by using early returns.
File: v2-core/src/abstracts/SablierV2Lockup.sol
332 function withdraw(
333 uint256 streamId,
334 address to,
335 uint128 amount
336 )
337 public
338 override
339 noDelegateCall
340 notNull(streamId)
341 updateMetadata(streamId)
342 {
343 // Check: the stream is not depleted.
344 if (_streams[streamId].isDepleted) {
345 revert Errors.SablierV2Lockup_StreamDepleted(streamId);
346 }
347
348 // Check: the withdrawal address is not zero.
349 if (to == address(0)) {
350 revert Errors.SablierV2Lockup_WithdrawToZeroAddress(streamId);
351 }
352
353 // Check: the withdraw amount is not zero.
354 if (amount == 0) {
355 revert Errors.SablierV2Lockup_WithdrawAmountZero(streamId);
356 }
357
358 // Retrieve the recipient from storage.
359 address recipient = _ownerOf(streamId);
360
361 // Check: if `msg.sender` is neither the stream's recipient nor an approved third party, the withdrawal address
362 // must be the recipient.
363 if (to != recipient && !_isCallerStreamRecipientOrApproved(streamId)) {
364 revert Errors.SablierV2Lockup_WithdrawalAddressNotRecipient(streamId, msg.sender, to);
365 }
366
367 // Check: the withdraw amount is not greater than the withdrawable amount.
368 uint128 withdrawableAmount = _withdrawableAmountOf(streamId);
369 if (amount > withdrawableAmount) {
370 revert Errors.SablierV2Lockup_Overdraw(streamId, amount, withdrawableAmount);
371 }
372
373 // Retrieve the sender from storage.
374 address sender = _streams[streamId].sender;
375
376 // Effects and Interactions: make the withdrawal.
377 _withdraw(streamId, to, amount);
378
379 // Interaction: if `msg.sender` is not the recipient and the recipient is a contract, try to invoke the
380 // withdraw hook on it without reverting if the hook is not implemented, and also without bubbling up
381 // any potential revert.
382 if (msg.sender != recipient && recipient.code.length > 0) {
383 try ISablierV2Recipient(recipient).onLockupStreamWithdrawn({
384 streamId: streamId,
385 caller: msg.sender,
386 to: to,
387 amount: amount
388 }) { } catch { }
389 }
390
391 // Interaction: if `msg.sender` is not the sender, the sender is a contract and is different from the
392 // recipient, try to invoke the withdraw hook on it without reverting if the hook is not implemented, and also
393 // without bubbling up any potential revert.
394 if (msg.sender != sender && sender.code.length > 0 && sender != recipient) {
395 try ISablierV2Sender(sender).onLockupStreamWithdrawn({
396 streamId: streamId,
397 caller: msg.sender,
398 to: to,
399 amount: amount
400 }) { } catch { }
401 }
402 }
https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/abstracts/SablierV2Lockup.sol#L0:0
File: v2-core/src/libraries/Helpers.sol
146 function checkCreateLockupLinear(uint128 depositAmount, LockupLinear.Timestamps memory timestamps) internal view {
147 // Check: the deposit amount is not zero.
148 if (depositAmount == 0) {
149 revert Errors.SablierV2Lockup_DepositAmountZero();
150 }
151
152 // Check: the start time is not zero.
153 if (timestamps.start == 0) {
154 revert Errors.SablierV2Lockup_StartTimeZero();
155 }
156
157 // Since a cliff time of zero means there is no cliff, the following checks are performed only if it's not zero.
158 if (timestamps.cliff > 0) {
159 // Check: the start time is strictly less than the cliff time.
160 if (timestamps.start >= timestamps.cliff) {
161 revert Errors.SablierV2LockupLinear_StartTimeNotLessThanCliffTime(timestamps.start, timestamps.cliff);
162 }
163
164 // Check: the cliff time is strictly less than the end time.
165 if (timestamps.cliff >= timestamps.end) {
166 revert Errors.SablierV2LockupLinear_CliffTimeNotLessThanEndTime(timestamps.cliff, timestamps.end);
167 }
168 }
169
170 // Check: the start time is strictly less than the end time.
171 if (timestamps.start >= timestamps.end) {
172 revert Errors.SablierV2LockupLinear_StartTimeNotLessThanEndTime(timestamps.start, timestamps.end);
173 }
174
175 // Check: the end time is in the future.
176 uint40 blockTimestamp = uint40(block.timestamp);
177 if (blockTimestamp >= timestamps.end) {
178 revert Errors.SablierV2Lockup_EndTimeNotInTheFuture(blockTimestamp, timestamps.end);
179 }
180 }
https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/libraries/Helpers.sol#L0:0
Updates
Lead Judging Commences
inallhonesty about 1 year ago
Submission Judgement Published 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,65520 USDC / LOC
45,000 USDC
3,100 USDC
5,340 USDC
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.