diwakergupta / stacks-blockchain-tob-audit

GNU General Public License v3.0
0 stars 0 forks source link

Stacks HTTP protocol messages are susceptible to man-in-the-middle attacks #30

Open ESultanik opened 3 years ago

ESultanik commented 3 years ago

Description

Peer URLs and data URLs are validated using util/strings.rs:

https://github.com/trailofbits/x-audit-blockstack-core/blob/d35ef465e9fa2ce327a181117f8ca7933b9df075/src/util/strings.rs#L286-L316

This validation code allows both http and https URLs, and is used for data_urls in peer messaging.

Moreover, StacksHTTPMessages are not signed, and their validation is a no-op:

https://github.com/trailofbits/x-audit-blockstack-core/blob/d35ef465e9fa2ce327a181117f8ca7933b9df075/src/net/http.rs#L2596-L2600

Exploit Scenario

Alice has access to network traffic between Bob and his peers. Alice can read the content of his Bob's messages, selectively choose to drop them based off of the content, and/or alter replies.

Recommendations

Enforce a messaging protocol that guarantees data integrity, e.g., by tunneling the Stacks HTTP protocol over TLS.

jcnelson commented 3 years ago

We use the HTTP module to serve block and microblock data, and to handle RESTful API requests.

Regarding the former use-case for block/microblock transmission, the peer already knows the hash of the blocks it fetches, and validates them against these hashes in src/net/downloads.rs.

Now, a man-in-the-middle could truncate a microblock stream while ensuring that the microblocks (and its underlying packet representation) are still well-formed, but this attack is detected by the above validation code.

Alice has access to network traffic between Bob and his peers. Alice can read the content of his Bob's messages, selectively choose to drop them based off of the content, and/or alter replies.

Alice reading the blocks and microblocks Bob downloads is not a concern -- this is public data. In fact, we deliberately avoided TLS in order to make it trivial for CDNs and proxies to cache block and microblock data (which is desirable, since it improves the availability of the blockchain data). Alice cannot trick Bob into accepting corrupt blocks and microblocks, nor can she trick him into accepting a truncated microblock stream due to these extra checks. If Alice is able to corrupt a block or microblock request/response sent between Bob and some remote node Charlie, then HTTPS isn't going to help Bob make progress in processing the blockchain -- Alice can disrupt a TLS packet stream just as easily as a non-TLS packet stream.

The remaining RESTful API endpoints are used by clients of the Stacks blockchain, such as explorers and wallets. Explorers and other "heavy" consumers of this API are expected to run a node locally anyway to improve access latency, obviating the need for TLS. When we deploy the Stacks node as an API endpoint for consumption by downstream clients who do not run their own nodes (like wallets), we put it behind a Nginx server that proxies all data over TLS.

ESultanik commented 3 years ago

Alice can disrupt a TLS packet stream just as easily as a non-TLS packet stream

True, but the concern here is that without TLS, Alice can inspect the message payloads and selectively choose to malform or drop specific messages. This could be harder to detect, and might enable a more subtle denial of service.