Skip to content

[Incident Report #0] Invisible WASM Admin Update

Jump to bottom
noah edited this page Feb 27, 2023 · 4 revisions

At 1:36 PM PST on 02/20/2023, a bug was discovered in the DAO DAO UI that allowed CosmWasm UpdateAdmin and ClearAdmin messages to be included in proposals without being displayed. This would allow someone to sneak a proposal through DAO governance that could arbitrarily change the admin of any smart contract that the DAO was currently the admin of without the voters knowing.

The bug was fixed and deployed by 2:58 PM.

We have no reason to believe this vulnerability was successfully exploited.

Timeline

All times in PST

  • 1:36 PM: pupmos notifies the DAO DAO team about the UI vulnerability, sending this proposal as proof
  • 1:48 PM: noah sees and acknowledges the issue
  • 1:58 PM: noah opens PR #1133 to fix the issue, which is merged into the development branch soon after
  • 2:10 PM: noah opens PR #1134 to merge development into main and deploy #1133 to mainnet, which gets merged to begin deploying merge commit 9ddfc05b504da7a6d5649a097e06671c595ad396
  • 2:58 PM: fix is deployed and noah notifies pupmos and DAO DAO team

Vulnerability analysis

We pass all messages in a proposal through a message decoder that detects if a message has content to decode (based on its type) and decodes it so the UI can display what the message does. Specifically, it looks for wasm messages and decodes their base64-encoded msg field, as well as stargate messages to decode their protobuf types. All other messages are left alone.

The cause was bad message decoding logic. All wasm messages EXCEPT update_admin and clear_admin contain a msg to be decoded. The decoder created a decodedMessageArray that got populated with all messages except wasm messages without a msg inside. The author forgot to add the original message to the decodedMessageArray in the case that there was no msg to be decoded (i.e. for all update_admin and clear_admin messages). At the end of the function, the decodedMessageArray was returned if it contained any messages, and the original msgs (non-decoded input array) was returned otherwise. This worked as intended when all messages were not update_admin nor clear_admin, or when all messages were only one of those two. But when one of those admin wasm messages was combined with any other non-wasm message OR non-admin wasm message, they were excluded from the final decoded message array. Thus, the admin wasm actions could be snuck through when paired with other messages.

Impact investigation

Before the fix was deployed, 102 proposals existed with one of the two admin messages. Of those, 92 were vulnerable because they contained other messages as well. However, 87 of those were proposals in legacy DAOs, and the legacy UI properly displayed these messages, so those were not affected. Also, 86 of those contained v1, migrate, migration, upgrade, or update, indicating that they were likely legacy => v1 DAO migrations.

The remaining 5 vulnerable proposals were:

The first two do not appear to be exploits, and the next two are test proposals used to confirm the vulnerability's existence by pupmos, and the last one appears to be a failed exploit against RED DAO by pupmos. In conclusion, we have no reason to believe that a successful exploit occurred before this vulnerability was patched.

Clone this wiki locally