Closed code423n4 closed 2 years ago
https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L337 https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L343 https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L359 https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L367 https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L374
Issue: fillOrder is a payable function, yet msg.value is only handled in certain paths within the function.
fillOrder
msg.value
Consequence: Sending ETH via msg.value in any of the following contexts will result in user funds being locked in the contract and lost.
Mitigation: Add checks in each of these call paths requiring msg.value to be 0.
Move to 2 (Med Risk) label A part duplicate of #226
2 (Med Risk)
Duplicate: Native ETH can be lost if it’s not utilised in exercise and fillOrder: https://github.com/code-423n4/2022-06-putty-findings/issues/226
Lines of code
https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L337 https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L343 https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L359 https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L367 https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L374
Vulnerability details
Issue:
fillOrder
is a payable function, yetmsg.value
is only handled in certain paths within the function.Consequence: Sending ETH via
msg.value
in any of the following contexts will result in user funds being locked in the contract and lost.Mitigation: Add checks in each of these call paths requiring
msg.value
to be 0.