Open code423n4 opened 2 years ago
I think this is valid! The free trial approach is indeed a risk on that front and we need to "warn" lock managers about this more.
For lock manager who still want to offer free trials, the best approach would probably be to set a high transfer fee to make sure that free trials cannot be transfered.
As a consequence of this I am not sure this is as critical as indicated by the submitter.
Nice find!
From what I can tell at least, this does seem like a viable attack vector. Can I ask why this should not be treated as high
risk? @julien51
Sorry for the long delay here. In short: this is valid, but only an issue for locks which are enabling free trials (no one has done it) and we would make sure our UI shows this as a potential issue. In other words: a lock manager would need to explicitly enable free trials, despite our warning to put their own funds at risk. For that reason I don't think this is "High".
While this is a valid issue pertaining only to lock managers who explicitly enable free trials, this may still lead to a loss of funds if cancelAndRefund
is called by a user who has transferred their time to another account. I still believe this deserves a high
severity rating.
In my honest opinion, a warning isn't sufficient to prevent such abuse. I think on-chain enforcement ideal in this situation.
Handle
WatchPug
Vulnerability details
The current design/implementation of freeTrial allows users to get full refund before the freeTrial ends. Plus, a user can transfer partial of thier time to another user using
shareKey
.This makes it possible for the attacker to steal from the protocol by transferring freeTrial time from multiple addresses to one address and adding up to
expirationDuration
and call refund to steal from the protocol.PoC
Given:
keyPrice
is 1 ETH;expirationDuration
is 360 days;freeTrialLength
is 31 days.The attacker can create two wallet addresses: Alice and Bob.
purchase()
, transfer 30 days viashareKey()
to Bob, then callscancelAndRefund()
to get full refund; Repeat 12 times;cancelAndRefund()
and get 1 ETH.Recommendation
Consider disabling
cancelAndRefund()
for users who transferred time to another user.