code-423n4 / 2023-11-kelp-findings

13 stars 11 forks source link

The lack of checks results in the possibility of duplicate addition of contract addresses or token addresses. #272

Open c4-submissions opened 10 months ago

c4-submissions commented 10 months ago

Lines of code

https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/LRTConfig.sol#L156 https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/LRTConfig.sol#L172

Vulnerability details

Impact

The _setContract and _setToken functions only verify whether tokenMap[key] == val and contractMap[key] == val already exist, without checking whether tokenMap[x] == val and contractMap[y] == val. In other words, it does not validate whether val already exists, where x and y represent values that were previously set in tokenMap and contractMap.This leads to the potential for duplicate settings of the assetAddress parameter and the contractAddress parameter, consequently giving rise to a series of issues.

Proof of Concept

https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/LRTConfig.sol#L156 function _setToken(bytes32 key, address val) private { UtilLib.checkNonZeroAddress(val); if (tokenMap[key] == val) { revert ValueAlreadyInUse(); } tokenMap[key] = val; emit SetToken(key, val); }

https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/LRTConfig.sol#L172 function _setContract(bytes32 key, address val) private { UtilLib.checkNonZeroAddress(val); if (contractMap[key] == val) { revert ValueAlreadyInUse(); } contractMap[key] = val; emit SetContract(key, val); }

Tools Used

vs

Recommended Mitigation Steps

add check

Assessed type

Context

c4-pre-sort commented 10 months ago

raymondfam marked the issue as insufficient quality report

c4-pre-sort commented 10 months ago

raymondfam marked the issue as duplicate of #69

c4-judge commented 10 months ago

fatherGoose1 changed the severity to QA (Quality Assurance)

c4-judge commented 10 months ago

fatherGoose1 marked the issue as grade-b