sherlock-audit / 2024-08-cork-protocol-judging

2 stars 2 forks source link

Kind Cornflower Horse - vulnerable assignment of arrLen variable. #307

Closed sherlock-admin3 closed 2 months ago

sherlock-admin3 commented 2 months ago

Kind Cornflower Horse

Low/Info

vulnerable assignment of arrLen variable.

Summary

  1. The order in which the assignment occurs might be a pre-assignment fault in a way that the intended assignment should be post-assigned instead.

Vulnerability Detail

  1. arrLen is assigned followed by lines where the dev. codes a possible case where end = _assets.length if end > _assets.length thus changes the value of end.
  2. We can see that uint256 arrLen = end - start. however arrLen will not be updated these updates regarding the value change of end;

Impact

  1. view function / low, or QA

Code Snippet

  1. poc

Tool used

Manual Review

Recommendation

diff --git a/Depeg-swap/contracts/core/assets/AssetFactory.sol b/Depeg-swap/contracts/core/assets/AssetFactory.sol
index c3a2091..8f74e64 100644
--- a/Depeg-swap/contracts/core/assets/AssetFactory.sol
+++ b/Depeg-swap/contracts/core/assets/AssetFactory.sol
@@ -66,7 +66,6 @@ contract AssetFactory is IAssetFactory, OwnableUpgradeable, UUPSUpgradeable {
     {
         uint256 start = uint256(page) * uint256(limit);
         uint256 end = start + uint256(limit);
-        uint256 arrLen = end - start;

         if (end > idx) {
             end = idx;
@@ -76,6 +75,8 @@ contract AssetFactory is IAssetFactory, OwnableUpgradeable, UUPSUpgradeable {
             return (ra, lv);
         }

+        uint256 arrLen = end - start;
+
         ra = new address[](arrLen);
         lv = new address[](arrLen);

@@ -108,7 +109,6 @@ contract AssetFactory is IAssetFactory, OwnableUpgradeable, UUPSUpgradeable {

         uint256 start = uint256(page) * uint256(limit);
         uint256 end = start + uint256(limit);
-        uint256 arrLen = end - start;

         if (end > _assets.length) {
             end = _assets.length;
@@ -118,6 +118,8 @@ contract AssetFactory is IAssetFactory, OwnableUpgradeable, UUPSUpgradeable {
             return (ct, ds);
         }

+        uint256 arrLen = end - start;
+
         ct = new address[](arrLen);
         ds = new address[](arrLen);
sherlock-admin2 commented 1 month ago

The protocol team fixed this issue in the following PRs/commits: https://github.com/Cork-Technology/Depeg-swap/pull/86