Fee is set by an admin and can be set maliciously to steal the funds that are entitled to go to the user.
Impact
Fee can be set to a maliciously high value to unfairly extract funds from protocol users. An owner can buy options, set fee to 100% and exercise options, paying only premium for tokens locked.
Additionally, the fee can be set to more than 100% causing a DoS of the exercise function, completely breaking contract's purpose. (the revertion happens when expression msg.value - fee underflows)
Tools Used
Manual analysis
Recommended Mitigation Steps
Set an upper limit to the fee not to go unconstrained.
Add an agreedFee parameter to createVault(...) arguments and require it is the fee in the storage, then save it as a vault parameter.
Lines of code
https://github.com/code-423n4/2022-05-cally/blob/main/contracts/src/Cally.sol#L117-L121
Vulnerability details
Fee is set by an admin and can be set maliciously to steal the funds that are entitled to go to the user.
Impact
Fee can be set to a maliciously high value to unfairly extract funds from protocol users. An owner can buy options, set fee to 100% and exercise options, paying only premium for tokens locked.
Additionally, the fee can be set to more than 100% causing a DoS of the
exercise
function, completely breaking contract's purpose. (the revertion happens when expressionmsg.value - fee
underflows)Tools Used
Manual analysis
Recommended Mitigation Steps
Set an upper limit to the fee not to go unconstrained.
Add an
agreedFee
parameter tocreateVault(...)
arguments and require it is the fee in the storage, then save it as a vault parameter.