code-423n4 / 2023-06-lukso-findings

3 stars 1 forks source link

msg-value-loop #71

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-06-lukso/contracts/LSP6KeyManager/LSP6KeyManagerCore.sol#L145-L175 https://github.com/code-423n4/2023-06-lukso/contracts/LSP6KeyManager/LSP6KeyManagerCore.sol#L192-L234

Vulnerability details

Impact

The use of msg.value in a loop in different places in the contract can introduce potential risks. The contract accepts Ether as part of its execution and relay calls. If the contract's logic does not handle or validate the msg.value appropriately, it can result in unintended Ether transfers. For example, if the contract expects a specific amount of Ether for a particular operation and msg.value is not properly validated, it can lead to Ether being locked in the contract or transferred incorrectly.

Proof of Concept

LSP6KeyManagerCore.executeBatch(uint256[],bytes[]) use msg.value in a loop: revert LSP6BatchInsufficientValueSent(uint256,uint256)(totalValues,msg.value)

https://github.com/code-423n4/2023-06-lukso/contracts/LSP6KeyManager/LSP6KeyManagerCore.sol#L145-L175

LSP6KeyManagerCore.executeBatch(uint256[],bytes[]) use msg.value in a loop: (totalValues += values[ii]) > msg.value

https://github.com/code-423n4/2023-06-lukso/contracts/LSP6KeyManager/LSP6KeyManagerCore.sol#L145-L175

LSP6KeyManagerCore.executeRelayCallBatch(bytes[],uint256[],uint256[],uint256[],bytes[]) use msg.value in a loop: (totalValues += values[ii]) > msg.value

https://github.com/code-423n4/2023-06-lukso/contracts/LSP6KeyManager/LSP6KeyManagerCore.sol#L192-L234

LSP6KeyManagerCore.executeRelayCallBatch(bytes[],uint256[],uint256[],uint256[],bytes[]) use msg.value in a loop: revert LSP6BatchInsufficientValueSent(uint256,uint256)(totalValues,msg.value)

https://github.com/code-423n4/2023-06-lukso/contracts/LSP6KeyManager/LSP6KeyManagerCore.sol#L192-L234

Tools Used

Manual analysis

Recommended Mitigation Steps

Assessed type

Loop

c4-pre-sort commented 1 year ago

minhquanym marked the issue as primary issue

c4-pre-sort commented 1 year ago

minhquanym marked the issue as low quality report

minhquanym commented 1 year ago

Seems invalid as msg.value is intentionally used in the executeBatch() loop

c4-sponsor commented 1 year ago

CJ42 marked the issue as sponsor disputed

c4-judge commented 1 year ago

trust1995 marked the issue as unsatisfactory: Invalid