Client: Client-side checks for smart contract code integrity

Created on 11 Nov 2018  路  2Comments  路  Source: aragon/client

From @osarrouy:

If I deploy a custom version of the ACL which do not throw some event to hide some permissions - or just deploy a no event version of the ACL before updating the standard one
Then I could give me all right on the finance app without this right being visible in the permission app right?

Currently, DAO creators can modify some critical apps in the DAO or create the DAO with some malicious apps with a custom kit, and the client will show that DAO as a regular DAO, misleading users into thinking they can trust the organization and its permissions.

These are some actionable steps for ensuring smart contract integrity. If any of these checks fail, the client should scream at the user every time they use the DAO, similar to how browsers alert about an invalid HTTPS cert. The items are mainly independent and can be rolled out progressively after we have the warning screen.

Kernel integrity:

  • Get the code of the kernel contract and check its hash is in a whitelist of known KernelProxy bytecode.
  • Get the current implementation of the kernel and check kernelRepo.getLatestVersionForContractAddress(implementation) is not zero (This will require publishing the Kernel to an APM repo, which we currently don't do).

App integrity:

  • Show a warning in the app sidebar if the appId isn't in the top-level registry ([app-name].aragonpm.eth).
  • Check that the appIds for frontend-less apps are in the top-level registry (ACL, EVMScriptRegistry and Vault).
  • Check that the default apps actually match their appId, or another appId in the top-level registry.
  • Get the code of all app proxies and check their hashes are in a whitelist of AppProxyUpgradeable and AppProxyPinned bytecode.
  • Get the implementation and check appRepo.getLatestVersionForContractAddress(implementation) is not zero (specially important for frontend-less apps). Will also require publishing ACL and EVMScriptRegistry to APM.
  • Any radspec string regarding an app upgrade (for Voting mainly) or setting a default app, should perform the above check to ensure the new implementation code is in the APM repo of the app.

Bonus:

  • History integrity: As explained in https://github.com/aragon/aragon/issues/462#issuecomment-437709314 none of the above checks can be trusted unless the history of Kernel and app versions is checked (as an old malicious version can introduce a backdoor in storage without emitting a log). We need to make sure that all implementations the kernel proxy was ever pointed to are in the Kernel APM repo, and (once the history of app upgrades can be trusted because the kernel integrity is checked) app proxies were only pointed to implementations in their APM repos.
  • Token Manager: Check the MiniMe token code hash is whitelisted.
  • Allow the user to see the code any app is currently running and perform a client-side code verification (fetch artifact.json and code.sol and compile the code in the browser before showing it to the user), if verification fails display a warning.
enhancement request security

Most helpful comment

To keep track of the history of kernel implementations in the KernelProxy itself.

This can't happen in storage as the malicious Kernel implementation could clean up the relevant storage in the proxy. The only possibility is for the KernelProxy to always log its implementation code before doing delegateFwd, as logs cannot be tampered with once emitted. The downside of this would be that checking Kernel integrity would involve checking tons of logs, but this is a process that can be started in the background and in case an issue is detected show a warning. Also this is something users would only need to do once.

All 2 comments

Hey,

Thanks a lot @izqui for such a clear summary of our discussion.

To complete: I think the integrity-checking process should apply both to the current version of apps but also to their older versions.

Example: if I grant myself a permission through a non event-emitter version of the ACL and then update it to the 'normal' version i can still 'trick' the permission app while the current version of the ACL is completely legit.

This is some kind of an infinite-regression since at some point I still have to trust the source of knowledge regarding the history of apps - _i.e._ the kernel - whose history can also be tricked ...

The solution I see there is :

  1. To keep track of the history of kernel implementations in the KernelProxy itself.
  2. To make it so that new implementations of the kernel are deployed from the KernelProxy itself - to make sure this history is consistent and that the proxy itself has not been deployed after the hack has been introduced to 'rewrite' history.

I'm not sure that's a super cool solution but I think it's worth thinking about it, especially for DAICO-like use case: otherwise the initiators of the DAO could easily leave with the funds ... which is not a super good news !

To keep track of the history of kernel implementations in the KernelProxy itself.

This can't happen in storage as the malicious Kernel implementation could clean up the relevant storage in the proxy. The only possibility is for the KernelProxy to always log its implementation code before doing delegateFwd, as logs cannot be tampered with once emitted. The downside of this would be that checking Kernel integrity would involve checking tons of logs, but this is a process that can be started in the background and in case an issue is detected show a warning. Also this is something users would only need to do once.

Was this page helpful?
0 / 5 - 0 ratings