How our Developer Community is helping us make Polymesh more robust and secure

Polymesh is the blockchain software at the core of Polymath, and the infrastructure that we’re building for financial securities market. Polymesh must be secure, robust, and free of bugs.

As the team building Polymesh, we do many things to make sure Polymesh is secure and robust. Some of the things we do are:

Automated Testing: We ensure that all of our features have associated unit tests, and integration tests where appropriate. This makes sure the new features we add don’t introduce new bugs.Independent Code Audit: To increase confidence in the security of our software, we’ve engaged the services of independent code auditors who specialize in blockchains and the Substrate stack. As per the reports of these audits, Polymesh is becoming better each day.ITN: Our incentivized testnet (ITN) is our key testing tool, which lets us receive lots of product feedback, helping us improve Polymesh. It also ensures and showcases that our software works at scale.

Since we can do only so much internally, we went ahead and asked our developer community to help us find bugs, and started the Polymesh Bug Bounty Program.

In this program, Polymesh developers from all around the world, get rewarded for finding legitimate security and performance bugs in Polymesh.

In this blog post, I want to highlight some issues around the management of keys, that the Bug Bounty program has helped highlight, and which we have fixed.

As a feature of the account management system, you can add additional secondary keys, representing other users (their identities), to your account and assign them different roles and permissions.

For example, if you want to add an auditor to your account, you can add their identity to your account, give them necessary permissions, and their keys will be added as secondary keys to your account, while you remain holder of the primary key associated with the account.

For the sake of understanding this issue, let’s call the primary key holder as A, and secondary key holder as B.

One of the bugs that were found showed that if an authorized secondary key holder tried to rotate a primary key, they will be able to take over the account itself.

In our example, if B tried to rotate (regenerate) the primary key on A’s account, then:

A’s primary key would be removed and become unassigned.B’s key (the secondary key) becomes the new primary key, while being the secondary key too.

In other words, this meant that if you had added someone to your account with their secondary key, and gave them the right permissions, they would be able to remove you as the primary holder of the account.

To solve this, while assigning a new primary key, our code now checks that the new key is not associated with any other identity.

This fix is reflected in this PR: https://github.com/PolymathNetwork/Polymesh/pull/1006

When you try to remove secondary keys, they are not removed from the DID records. This means that the permissions associated with the key are not removed, the key will also remain in the list of secondary keys.

The reason it was happening was that our code was using iterators. A set of keys to be deleted was being stored in an iterator, and the way our code was set up, it had to go through the iterator many times to delete all the keys required to be deleted, but since it was an iterator, it could only be traverse once.

We found that the issues were with the way we were doing comparison and sorting, so we improved on that. Along with that, we stopped using iterators, and instead are now using a list of keys.

Because of the issues with removing secondary keys that we just discussed, in some edge cases our users were able to add the same Secondary key more than once, ending up duplicating the keys.

Not just that, all these duplicated entries of the same secondary key would have different permissions. This would have confused our systems and would have given errors and performance issues to our users.

We re-wrote some parts of our code to perform better checks when a key is being deleted and/or created and added, to make sure that the secondary keys are not duplicated.

This change, and the fix for the previous issue related to secondary keys, is highlighted in this PR: https://github.com/PolymathNetwork/Polymesh/pull/1018