Polygon Lack Of Balance Check Bugfix Postmortem ($2200000 USD)

We discussed this vulnerability during Episode 109 on 10 January 2022

Polygon places the blame for this bug on not checking that the from address in a transfer actually has the balance to cover the transfer in the first-place. While I don’t doubt that as a core issue it feels like that may only be part of the issue, the other part being a lack of error checking, or perhaps improper error handling.

Getting into the actual issue, the root issue is that the it is never checked if from is able to fulfil the transfer. Beyond that it seems that error handling might also be an issue, in that transferWithSig does not check if ecrecovery returned an appropiate address, as it returns address(0x0) on error conditions. It also seems noteworthy that ecrecovery itself only raises an error its call to ecrecover fails and returns 0x0. I’m not too familiar with dapp development so I’m not sure if this is expected but it seems that it should be raising an error in all error situations rather than just returning a special value.

So, as exrecovery returns address(0x0) without any other error indicator when the length of the hash is not 65 is can very easily be triggered. transferWithSig will use this address as the from address, resulting in a transfer from the “genesis contract” of any amount.

Patch: This was resolved by switching to a fork and entirely disabling the transferWithSig function.