Platform: Code4rena
Start Date: 13/01/2022
Pot Size: $75,000 USDC
Total HM: 9
Participants: 27
Period: 7 days
Judge: leastwood
Total Solo HM: 5
Id: 73
League: ETH
Rank: 15/27
Findings: 1
Award: $207.54
🌟 Selected for report: 1
🚀 Solo Findings: 0
1.4926 LPT - $55.34
152.2025 USDC - $152.20
kemmio
Vulnerability in requireValidMigration() function gives opportunity to authenticate on behalf of ZERO address (l1addr == ZERO) and migrate locked up bonds, delegators, sender
L1Migrator contract's functions migrateDelegator(), migrateUnbondingLocks(), migrateSender() use requireValidMigration() to authenticate the migration request, as can be seen in: https://github.com/livepeer/arbitrum-lpt-bridge/blob/main/contracts/L1/gateway/L1Migrator.sol#L164-L173 https://github.com/livepeer/arbitrum-lpt-bridge/blob/main/contracts/L1/gateway/L1Migrator.sol#L214-L228 https://github.com/livepeer/arbitrum-lpt-bridge/blob/main/contracts/L1/gateway/L1Migrator.sol#L267-L274
requireValidMigration() checks if l2addr=ZERO and reverts in that's the case: https://github.com/livepeer/arbitrum-lpt-bridge/blob/main/contracts/L1/gateway/L1Migrator.sol#L506-L509
Next it checks wether msg.sender==l1addr or tries to authenticate with signature otherwise: https://github.com/livepeer/arbitrum-lpt-bridge/blob/main/contracts/L1/gateway/L1Migrator.sol#L510-L514
It calls recoverSigner() for that purpose which calls ECDS.recover to recover signing address, but before that it checks if signature is empty and returns address(0): https://github.com/livepeer/arbitrum-lpt-bridge/blob/main/contracts/L1/gateway/L1Migrator.sol#L522-L524
This functionality can be abused to bypass authentication for ZERO address
Proof of Concept: (add this tests to ./test/unit/L1/l1Migrator.test.ts and run "yarn test test/unit/L1/l1Migrator.test.ts" )
it('migrates delegator for l1addr==ZERO auth', async () => { const sig = '0x'; let tx = l1Migrator .connect(notL1EOA) .migrateDelegator('0x0000000000000000000000000000000000000000', l1EOA.address, '0x', 0, 0, 0, { value: ethers.utils.parseEther('1'), }); await expect(tx).to.emit(l1Migrator,'MigrateDelegatorInitiated'); }); it('migrates unbonding locks for l1addr==ZERO auth', async () => { const sig = '0x'; let tx = l1Migrator .connect(notL1EOA) .migrateUnbondingLocks( '0x0000000000000000000000000000000000000000', l1EOA.address, [], '0x', 0, 0, 0, { value: ethers.utils.parseEther('1'), }, ); await expect(tx).to.emit(l1Migrator,'MigrateUnbondingLocksInitiated'); }); it('migrates sender for l1addr==ZERO auth', async () => { const sig = '0x'; let tx = l1Migrator .connect(notL1EOA) .migrateSender('0x0000000000000000000000000000000000000000', l1EOA.address, '0x', 0, 0, 0, { value: ethers.utils.parseEther('1'), }); await expect(tx).to.emit(l1Migrator,'MigrateSenderInitiated'); });
Remove these lines: https://github.com/livepeer/arbitrum-lpt-bridge/blob/main/contracts/L1/gateway/L1Migrator.sol#L522-L524
#0 - yondonfu
2022-01-24T02:21:50Z
Severity: 1 (Low)
The zero address won't have any relevant state in the L1 contracts so a requireValidMigration bypass wouldn't result in a change in assets that the zero address is authorized to move on L2.
#1 - 0xleastwood
2022-01-29T11:31:02Z
Agree with sponsor, this doesn't seem exploitable.
#2 - yondonfu
2022-01-31T21:19:03Z