Multiple pairs of the same tokens can get created at `createLendgine` and saved in `getLendgine`

[code423n4]

Lines of code

https://github.com/code-423n4/2023-01-numoen/blob/c75b39dc93fe327039b097ba149e1abe90d3b2f5/src/core/Factory.sol#L63-L88

Vulnerability details

Multiple pairs of the same tokens can get created at createLendgine and saved in getLendgine

Summary

Multiple pairs of the same tokens can get created at createLendgine even if that does not make sense. This can affect frontends that read this value.

Vulnerability Detail

A pair is expected to have 2 tokens, these 2 tokens would have their own properties that will always remain the same, a token with 18 decimals has 18 decimals and that's it.

However, here we have an interesting scenario. Code is based in these as seen in IFactory in Uniswap contract and Primitive contract.

There they check that pair is already created by checking a mapping with just token0 and token1: Uniswap, Primitive.

In createLendgine it is being used a mapping not just with both token addresses but with tokenExps and upperBound. createLendgine is external and has no access control.

if (getLendgine[token0][token1][token0Exp][token1Exp][upperBound] != address(0)) revert DeployedError();

If some frontend is reading all the instances on the mapping, they can get duplicate of pairs that won't behave as they should.

A malicious actor or just by error while creating a Lendgine, can lead to creating a lot of Lendgines with all the combinations of values of token1Exp, token2Exp from 6 to 18 and different upperBounds. The amount of pairs of the same tokens that can be created is exponential as upperBounds is uint256.

Impact

Bad pairs being created, overwhelming frontends with multiple pairs for the same tokens creating a source of DoS, users using a pair with wrong tokenExp.

Code Snippet

https://github.com/code-423n4/2023-01-numoen/blob/c75b39dc93fe327039b097ba149e1abe90d3b2f5/src/core/Factory.sol#L63-L88

Tool used

Manual Review

Recommendation

Consider using a mapping that check only pairs of tokens and using a source of access control. Also consider creating a way to delete wrong Lendgines

[c4-judge]

berndartmueller marked the issue as primary issue

[c4-sponsor]

kyscott18 marked the issue as sponsor acknowledged

[kyscott18]

While I see your point, it is up to the front end providers and the end users to make sure that they are interacting with a legitimate Lendgine. We wanted to leave this flexibility open for cases like shib which are really low in value, and any future use cases that arise

[berndartmueller]

According to the C4 judging criteria:

2 — Med: Assets not at direct risk, but the function of the protocol or its availability could be impacted, or leak value with a hypothetical attack path with stated assumptions, but external requirements.

None of these risks apply. Therefore I consider QA (NC) to be appropriate.

[c4-judge]

berndartmueller changed the severity to QA (Quality Assurance)

[c4-judge]

berndartmueller marked the issue as grade-c