014.md

zaskoh

medium

Possible re-entrency attack in Cooler if deployed with malicious factory

Summary

If Cooler is deployed with a malicious factory it is vulnerable to re-entrency attacks.

Vulnerability Detail

At the moment Cooler creates events on the factory via the factory.newEvent call. The call is done before important state changes and can moved down in the functions to adhere to the check effect pattern.

File: src/Cooler.sol
81:         factory.newEvent(reqID, CoolerFactory.Events.Request);
82:         requests.push(
83:             Request(amount, interest, loanToCollateral, duration, true)
84:         );

File: src/Cooler.sol
094:         factory.newEvent(reqID, CoolerFactory.Events.Rescind);
095: 
096:         Request storage req = requests[reqID];
097: 
098:         if (!req.active)
099:             revert Deactivated();
100:         
101:         req.active = false;

File: src/Cooler.sol
163:         Request storage req = requests[reqID];
164: 
165:         factory.newEvent(reqID, CoolerFactory.Events.Clear); 
166: 
167:         if (!req.active) 
168:             revert Deactivated();
169:         else req.active = false;

Impact

If factory is malicious it can reenter and manipulate the state of a request.

Code Snippet

https://github.com/ohmzeus/Cooler/blob/main/src/Cooler.sol#L81 https://github.com/ohmzeus/Cooler/blob/main/src/Cooler.sol#L94 https://github.com/ohmzeus/Cooler/blob/main/src/Cooler.sol#L165

Tool used

Manual Review

Recommendation

Move the factory.newEvent down in the functions, just before the transfer so the states are updated accordingly before the external call is made. Alternativls you can add a re-entrancy guard, but as the official factory has no possibility for a re-entrency attack it is not the best decision regarding gas effficiency.