Submitted by 0xA5DF, also found by 0xbepresent and ladboy233
The Caller contract enables users to authorize other users to execute tx on their behalf.
This option enables the authorized/delegated user to add more users to the authorized users list.
In case the original user is trying to remove an authorized user (i.e. run unauthorize()), the delegated user can simply front run that tx to add another user, then after unauthorized() is executed the delegated can use the added user to gain his authority back.
This would allow the malicious delegated user to keep executing txs on behalf of the original user and cause them a loss of funds (e.g. collecting funds on their behalf and sending it to the attackers address).
The test below demonstrates such a scenario:
Front running can be done either by sending a tx with a higher gas price (usually tx are ordered in a block by the gas price / total fee), or by paying an additional fee to the validator if they manage to run their tx without reverting (i.e. by sending additional ETH to block.coinbase, hoping validator will notice it).
Its true that Alice can run unauthorize() again and again and needs to succeed only once, but:
Dont allow authorized users to call authorize() on behalf of the original user.
This can be done by replacing _msgSender() with msg.sender at authorize(), if the devs want to enable authorizing by signing I think the best way would be to add a dedicated function for that (other ways to prevent calling authorize() from callAs() can increase gas cost for normal calls, esp. since well need to cover all edge cases of recursive calls with callBatched()).
xmxanuel (Drips) commented:
CodeSandwich (Drips) disagreed with severity and commented:
Alex the Entreprenerd (judge) decreased severity to Medium and commented:
xmxanuel commented:
Alex the Entreprenerd (judge) commented:
For this contest, 13 reports were submitted by wardens detailing low risk and non-critical issues. The report highlighted below by berndartmueller received the top score from the judge.
The following wardens also submitted reports: SleepingBugs,
rbserver,
hansfriese,
IllIllI,
0xSmartContract,
nalus,
zzzitron,
0xA5DF,
fs0c,
chaduke,
btk, and
HollaDieWaldfee
.
