How a smart contract vulnerability was discovered and fixed

As you guys know, the Incognito network is in active development – a combined effort from the core dev team, a growing community, as well as external contributors. As we continually push upgrades and fixes, we’ll also be keeping you guys in the loop so we can all have a clearer picture of how the network as a whole is getting stronger.

Last Friday (2020/09/25), Incognito paused shield and unshield functions for ETH/ERC20 after identifying a smart contract bug. Here’s a breakdown of what happened, and what we did to address it.

How did we find it?

An external smart contract auditor samczsun found an issue with the Incognito contract, and asked a core team member to prove contract ownership before disclosing the issue. We’d like to express our thanks to samczsun for his professionalism and carefulness in making sure the issue was not disclosed to a dishonest party.

A core team developer sent the auditor a transaction signed by the Incognito contract admin. The transaction memo contained the developer’s telegram id in order for the auditor to trust that he was talking to the right person. The auditor pointed out the vulnerable contract code to the developer, and the core development team confirmed the bug. The contract was then paused.

What was the bug?

The bug came from the executeMulti function. Basically, the function allows the user to call a function from another contract with passed params on multiple tokens in the same call. It differentiates from the execute function by only accepting a single token.

Unfortunately, the executeMulti function was lacking logic to validate for token duplication, potentially allowing one to build a custom exchange and duplicate entries to double spend currency.

How did we fix it?

The core team reviewed the contract and realized that the executeMulti function was extraneous and therefore safe to remove from the contract. We believe that this was the safest and fastest way to solve the problem. The fix was deployed to the new contract within the same hour of bug confirmation, and tokens were moved to the new contract successfully.

What’s next?

We call on everyone who is able and willing to help make Incognito as safe and robust as it can be. The core team kickstarted the network and has taken on the responsibility of giving it legs – but Incognito belongs to all of us, and none of us can do it alone. You don’t have to be a smart contract wizard or a cryptography expert - every bug report, new idea, or piece of relevant research is always welcome.

Thank you to everyone who works towards privacy for all.

22 Likes

Great post! I’m glad there’s someone out there that cared enough to help our project and the industry. Does incognito have a bug bounty program? Does incognito submit proposed contract upgrades to any 3rd party auditing firm before making them live?

3 Likes

hey @marko, we’re considering about bug bounty program officially. There are a few things we need to figure out: issue levels (critical, major, minor, …), system types (chain, contracts, app, …), reward amounts, … especially as everything is still under heavy development so things may change continually over time.

6 Likes

Was this a guy who found a bug randomly or an actual audit? I noticed there is no audit report on the smart contract, no official label either. The contract in fact was started by a personal eth address with some PUBG erc20 tokens on it. It would look more professional if it had proper labeling and would add us to the list of decentralized exchanges listed on etherscan. If there has never been a paid audit we should get one and submit its report on the contract label to establish trust.

Contract address: https://etherscan.io/address/0x97875355ef55ae35613029df8b1c8cf8f89c9066#code
nosec
Also I am not a solidity dev but there are these error’s present on the compiler:
Compiler specific version warnings:

The compiled contract might be susceptible to EmptyByteArrayCopy (medium-severity), DynamicArrayCleanup (medium-severity), MissingEscapingInFormatting (very low-severity), ArraySliceDynamicallyEncodedBaseType (very low-severity), ImplicitConstructorCallvalueCheck (very low-severity) Solidity Compiler Bugs.

6 Likes

How much money does the team make from the DAO ? They cannot afford a smart contract audit ? This smells bad

1 Like

They are in the process to do massive Audit. This is in the roadmap 2021

2 Likes

No… that’s not how things work actually… that’s how you lose all your users funds.

You create eth contract, and audit before deployment… not over 1 year after

If you don’t believe me I can literally reference you to every reputable eth smart contract made and it will follow this timeline. All that is needed is 1 bug and you can have EVERYTHING drained from the contract. Look at what happend to cover protocol:

https://www.binance.com/en/support/announcement/200da2a0f3c74706841a9214fd55c94d

3 Likes

Hey @Matt6412,

You may read the main discussion here: Security audit is a MUST!

2 Likes

Thanks for the reference. So that answers my question, NEVER been audited

1 Like

Internally yes, externally no but they will.

1 Like

A security researcher could have emptied the wallets of the smart contract, but chose NOT to and instead did the right thing. This is clear evidence no such internal audit has been done, or if so, said internal auditer is NOT qualified to do such a task.

Also, crypto is about being trustless, therefore proof is ALWAYS needed to verify over blind trust. That is the whole point of posting an external security audit (in this case I’m only talking about the eth bridge, not the whole project!) This gives me IMMENSE pause in regards to even the cryptography codebase of the core protocol being coded right / bug free.

There is over 25 million in liquidity pools and over 18 million USD value in shielded coins, ALL of which are at risk because of no outside audits

5 Likes

I agree with you on most of what you wrote but don’t agree on this:

Even 3rd party auditors cannot catch every breach and there are many cases too. I mean that some specialized firms are preferred but the corresponding bug may not show directly that the auditor isn’t qualified. This is the software world. I’m a senior computer engineer. I assure you that there is no software without any bugs. There are bugs waiting to be caught :joy: That’s why I had suggested an insurance fund on the very same thread.

Anyway, here are some related threads.

1- They really do internal audit: Internal Code Audit
2- The external audit is on agenda since Q1 2020. There are also other topics. This is one of them. 3rd Party Audits
3- It is on the roadmap of 2021: Incognito's 2021 privacy roadmap for the world

5 Likes

Thank you @abduraman this link really helps put my mind at somewhat of an ease

But yes I agree with you, 3rd party audit’s are crucial as bugs are VERY hard to find

3 Likes

I agree that this is a common practice but you should remember that Incognito is still under active development where we’re trying to add more features to it (i believe you don’t also want to see Incognito just likes thousands of blockchain projects out there - having a white paper or even building a blockchain, raising funds and no one uses) in order to bring more value to its users. And perhaps as you could see, we updated the smart contract a few times throughout last year to add more features besides the bridge. So according to the roadmap, we won’t have any significant updates this year against the smart contract so this point may the right time to make an external audit as @andrey is actively looking for audit companies - he already contacted a couple of companies and we’re considering to choose the right one.

@Fatfaa dont try to say such not constructive thing, this didn’t help at all as you don’t know the story behind the scene. It’s not about what DAO makes but what audit companies request and how they work for the audit. It must be a reasonable number along with an appropriate process…

3 Likes

@duc I’ve been involved with many projects… audits and security are always first priority before deployment, especially when it comes the core team holding all the assets in a centralized manner in which makes incognito EVEN MORE unique because you are operating and holding the private keys to all these users funds (every trusted bridge and smart contract).

This is essentially exchange level security needed, and we cant get a 3rd party audit for a smart contract? All it takes is just 1 small bug and poof, all 50 million of assets are GONE.

Theres 5 million premined prv and no governance whatsoever, no public plan for governance, no transparency, and a dao that has no rules disclosed to us. PRV has a market cap of over 30 million usd, I think ontop of the initial 1 million investment the core team made with profits, and the premined coins, a 3rd party audit can be afforded. Maybe even 3 or 4 audits. But therein lies the issue of transparency, which was brought up on the last prv holders call and not addressed.

3 Likes

can we get a legit audit done asap.

3 Likes

Hey guys. @Matt6412 @Interceptingfist we are in chat with coinspect.com, peckshield.cn/en and samczsun about a new audit for the bridge.

If you guys know other reputable firm we should work with, please share contacts.

8 Likes

thanks for the update!

2 Likes

So is there a reason I still can’t find any results of a 3rd party audit?

1 Like

Hey, we have been working with the Coinspect audit firm for months now, the audit is almost done, they reported a few findings (no critical security issue found so far) and we fixed them already. The next step is to wrap things up, deploy the new smart contracts for the fixes and then publish the audit report.

6 Likes