Detailed description of the impact of this finding.
Low:
The use of the 'payable' keyword on the addLiquidity function could allow an attacker to add excessive amounts of ether, potentially leading to an out-of-gas exception.
There is no input validation or bounds checking on the input parameters, making it vulnerable to malicious inputs.
Medium:
The contract uses the 'liveliness' modifier to check the deadline for transactions, however, it's possible that the deadline could be set to a block in the past, potentially allowing an attacker to bypass the deadline check.
There is no protection against reentrancy attacks, so an attacker could execute the addLiquidity function, before the balance checks and transfers have completed, potentially leading to a loss of funds.
High:
The code uses the msg.sender to check the sender of the transaction, however, this can be easily spoofed by an attacker. A better approach would be to use the caller address (e.g. msg.caller).
There is no protection against a front-running attack, where an attacker could manipulate the order of transactions to execute the addLiquidity function before a previous transaction has completed.
I recommend that this code be thoroughly reviewed and tested by a security expert before deployment to a production environment.
Lines of code
https://github.com/code-423n4/2023-01-numoen/blob/2ad9a73d793ea23a25a381faadc86ae0c8cb5913/src/periphery/LiquidityManager.sol#L20
Vulnerability details
Impact
Detailed description of the impact of this finding.
Low: The use of the 'payable' keyword on the addLiquidity function could allow an attacker to add excessive amounts of ether, potentially leading to an out-of-gas exception. There is no input validation or bounds checking on the input parameters, making it vulnerable to malicious inputs. Medium: The contract uses the 'liveliness' modifier to check the deadline for transactions, however, it's possible that the deadline could be set to a block in the past, potentially allowing an attacker to bypass the deadline check. There is no protection against reentrancy attacks, so an attacker could execute the addLiquidity function, before the balance checks and transfers have completed, potentially leading to a loss of funds. High: The code uses the msg.sender to check the sender of the transaction, however, this can be easily spoofed by an attacker. A better approach would be to use the caller address (e.g. msg.caller). There is no protection against a front-running attack, where an attacker could manipulate the order of transactions to execute the addLiquidity function before a previous transaction has completed. I recommend that this code be thoroughly reviewed and tested by a security expert before deployment to a production environment.