Platform: Code4rena
Start Date: 08/06/2022
Pot Size: $115,000 USDC
Total HM: 26
Participants: 72
Period: 11 days
Judge: leastwood
Total Solo HM: 14
Id: 132
League: ETH
Rank: 15/72
Findings: 2
Award: $1,311.43
🌟 Selected for report: 1
🚀 Solo Findings: 0
🌟 Selected for report: 0x52
Also found by: csanuragjain
Sponsor vaults drained
reimburseRelayerFees uses SponsorVault funds to repay users the fees they pay to relayers. A malicious relayer could create a large number of transactions with the max reimbursed relay fee specified in SponsorVault between chains for which they relay all of them. They would receive back the relay fee from the SponsorVault and they would also get the relay fee they paid themselves.
I don't see any way to mitigate this without substantial changes to the functionality of SponsorVault. Teams deploying SponsorVaults should be informed of potential misuse and set reimburse limits accordingly.
#0 - LayneHaber
2022-06-24T16:29:07Z
I'm not sure I agree with this as an issue -- the relayers are whitelisted (to prevent generalized front-running, and limiting relayers to reliable relay networks).
Usually any dusting systems like the reimburseRelayerFees do not have sybil resistance (unless you connect your socials or something similar), and this is something we will have to advertise to the people who create and fund the vault.
NOTE: inspiration for this feature was taken from existing nomad dusting, which has the same potential shortcomings, see here. Generally, chains have been willing to fund these vaults even with these drawbacks. That being said, balance checks could be added to add some slight guardrails!
#1 - LayneHaber
2022-06-27T15:20:28Z
#2 - 0xleastwood
2022-08-02T04:39:36Z
I actually agree with the warden here. This type of attack would not be recognisable and could be disguised as general protocol activity. However, users' funds are not at direct risk, but sponsors' funds who intentionally give up these funds would be at risk of leaking value. Downgrading to medium risk.
🌟 Selected for report: BowTiedWardens
Also found by: 0x1f8b, 0x29A, 0x52, 0xNazgul, 0xNineDec, 0xf15ers, 0xkatana, 0xmint, Chom, ElKu, Funen, IllIllI, JMukesh, Jujic, Kaiziron, Lambda, MiloTruck, Ruhum, SmartSek, SooYa, TerrierLover, TomJ, WatchPug, Waze, _Adam, asutorufos, auditor0517, bardamu, c3phas, catchup, cccz, ch13fd357r0y3r, cloudjunky, cmichel, cryptphi, csanuragjain, defsec, fatherOfBlocks, hansfriese, hyh, jayjonah8, joestakey, k, kenta, obtarian, oyc_109, robee, sach1r0, shenwilly, simon135, slywaters, sorrynotsorry, tintin, unforgiven, xiaoming90, zzzitron
142.2658 USDC - $142.27
MATIC transfers will revert
Native MATIC on Polygon has the contract address 0x0000000000000000000000000000000000001010, so when withdraw is called it will be treated as an ERC20 token. MATIC transfer requires that msg.value = value (https://polygonscan.com/address/0x0000000000000000000000000000000000001010#code L518). Since withdraw doesn't call transfer with a message value, the transfer will return false causing safeTransfer to revert.
Create exception for native MATIC in function
#0 - jakekidd
2022-06-24T21:33:39Z
Confirming issue. Might be best to have the native token address be a set value in an initializer, but the most gas efficient method would be to set it as a constant.
#1 - jakekidd
2022-06-24T22:04:36Z
Amending this evaluation: according to the design of the application and the intended use-case, any/all references to native token should be address(0). Simply passing in address(0) as the argument for token is sufficient to indicate usage of the native token.
If anything, this is a QA issue.
#2 - LayneHaber
2022-06-30T12:12:57Z
^^ correct, this is a convention (already being used in production on MATIC). The main concern is the treatment, which means on the frontend all native assets should use the address(0) convention.
The matic team is familiar with the convention already :) (and we did run into this issue in production with nxtp v0)
#3 - 0xleastwood
2022-08-15T00:26:23Z
As per the sponsor's comments, downgrading this to QA.