search

move-language / move

Apache License 2.0
2.25k stars 684 forks source link

[security] Cherry-picking recent security fixes from Aptos #950

Closed wrwg closed 1 year ago

wrwg commented 1 year ago

Security fixes from the last few weeks. This is a series of PRs addressing DoS attacks on the bytecode verifier. The attacks have been reported via the bug bounty program and each of them has been converted to a test. The general method of defense is a new so-called 'meter' in the bytecode verifier which counts complexity. For example, for absint, the number of joins and steps is counted and weighted.

dariorussi commented 1 year ago

this is pretty good, give us a little bit to review and thanks!

dariorussi commented 1 year ago

I like this overall, a lot. I think it captures well the different dimensions of looping and allocating that can be used as DOS attack. I am going to let others contribute until Monday when I will come and accept if not accepted sooner.

It seems there is some more to do but it feels it is a good direction for now and possibly for quite some time. Curious to hear if others have feelings about long term solutions and how this plugs in towards the final goal (whatever that may be)

wrwg commented 1 year ago

we tend to like the approach though we may want to change something in our repo and push it back as needed. Hope that is ok with you all. In general - for all Move community - thinking about ways to plug the metering in the least intrusive way should be a priority of some kind. It seems inevitable that different metering (or not at all) will be a reality moving forward. So something that allows all parties to move freely would be a good thing. Particularly considering this is the verifier...

If someone could answer @tnowacki's comments that would be appreciated

Sorry for the delay -- first traveling, then catching up. Yes, please feel free to massage and also push useful changes (like @tnowacki suggesting) upstream.