Postmortem: Polkadot 2.0.1 Release

1

This is a postmortem describing the reason for the polkadot-fellows/runtimes’s 2.0.1 fast runtime upgrade.

On Tuesday November 4th, towards the end of AHM, Parity and the fellowship received a security notice from the SRLabs audit team about a missing origin check in the election-provider-multi-block pallet. The vulnerability was due to a let _ = check_origin() mistake; not including the ? to raise an error in case of an origin failure, and silencing the warning with a _. The transaction ignoring the origin check was meant to be a privileged origin managing the pallet’s parameter via governance only, and the vulnerability would allow anyone to dispatch it.

Luckily, since Parity was already on-call on the said date due to AHM, the issue was quickly fixed, and back-ported to unstable-2507 release of pallet-election-provider-multi-block, currently published as version 0.3.4.

Afterwards, the 2.0.1 runtime release was made and thanks to the quick voting response from the fellowship and token-holders, both referendums passed with a quick turnaround. As I am writing this, both Polkadot and Kusama AH runtimes have been upgraded with this fix.

The vulnerable pallet is a highly domain specific one created solely to be used in Polkadot system chains. Yet, to be safe, we have yanked all previous and vulnerable versions of this crate in crates.io.

6 Likes
2

This looks like something could be detected by static analyzer tool or AI code review tool. It is pretty easy to setup some AI review bot to catch common known antipattern. All we need to do is document them all down and include them in the prompts. Maybe it is worth to look into this direction?

I don’t know if scout can already detect this particular case, but I don’t think it is hard to add such rule if not already there GitHub - CoinFabrik/scout-audit: Scout is an extensible open-source tool intended to assist smart contract developers and auditors detect common security issues and deviations from best practices. Scout audit is the core development on which we extend scout for specific blockchains.

1 Like
3

Yeah I fully agree, this stuff should not happen, even if it is part of a big PR that is hard to review (which was the case here). SRLabs integrated semgrep after this incident for such kind of testing. I think even a custom clippy rule can do it.