rust-lang / rust-lang/rust

type_id is not sufficiently collision-resistant

Open
#129,014 15 comments 16 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

C-bug C-discussion I-unsound T-compiler
Dominant language
Rust
Stars
119k
Forks
16.1k
PR merge metrics
PR metrics pending

Description

This is a re-post of https://github.com/rust-lang/rust/issues/10389, attempting to summarize the current state of the discussion since that issue had too many comments to still be useful.

The soundness of functions like downcast relies on the type_id of two different types never being equal. Currently, the type_id is a 128-bit hash of the full type identity, computed specifically via SipHash-1-3 with an all-zero key. This is not a strong enough hash function for this purpose.

The lang team decided that relying on a full (non-truncated) cryptographic hash is fine -- we don't have to guarantee soundness against an infinite-resource attacker that can generate collisions in cryptographic hash functions.
However, SipHash even in its default configuration (SipHash-2-4) is not a cryptographic hash, as clarified by its author[^siphash] -- it is a pseudo-random function (PRF); that means it assumes a secret key, but Rust hard-codes an all-zero key and in fact has no way to keep a key secret. By standards for cryptographic hash functions, SipHash-2-4 with an all-zero-key is weaker than MD5, and generating a collision for that is pretty easy these days. SipHash-1-3 is even weaker, we should thus expect it to be a matter of hours to create a collision, if someone really tried.

[^siphash]: In the original paper, they even write: "We comment that SipHash is not meant to be, and (obviously) is not, collision-resistant."

Generating a concrete example of an unsoundness from that is a bit more tricky since one would have to find a Rust type generating the collision, but it seems fairly clear that the bar of "full (non-truncated) cryptographic hash" is not met by the current type_id implementation. The point of this issue is to determine how the compiler implementation can best satisfy the soundness expectation set by the lang team.

If you instead want to argue that the lang team should change its mind, please open a new issue and gather arguments in favor of that position, so that a summary of all the arguments for either option can be brought to the lang team for discussion. Also, this issue is only about type_id. According to this comment, the only other case where a hash collision could cause unsoundness is with incremental builds; see https://github.com/rust-lang/rust/issues/129016 for the issue tracking that. All other hashes are actively checked for collisions by Rust. That said, it is not clear to me whether that also covers dynamic linking scenarios -- if someone knows about hash-related soundness concerns for that case, please file a new issue.

Possible solutions

We should do one of the following:

  1. switch to a stronger hash function, or
  2. switch to a different scheme that doesn't rely on collision-resistance of the hash function.

Worth noting is that a cryptographic hash function these days must output at least 256 bits to be considered worth its salt. The lang team has not spelled out their exact definition of "cryptographic hash function", but the standard definition includes collision resistance, and in fact collisions are exactly what we are most worried about here, so it seems reasonable to assume that this is part of the lang team intent. A lang team member mentioned BLAKE3 as a candidate, further corroborating this claim. So we'd have to switch to BLAKE3 or SHA2 or something like that.

Therefore, in both cases will we need more than the currently available 128 bits of a TypeId to obtain the desired level of collision resistance. To avoid further increasing the size of TypeId, the most likely scheme would be to include a pointer in TypeId that points to the remaining information -- either a 256-bit hash, or a string (null-terminated or with leading length information, to obtain a "thin" pointer), or something like that.

https://github.com/rust-lang/rust/pull/95845 explores what this could look like without depending on a hash function: TypeId becomes a pair of a (not-soundness-critical) hash and a pointer. If the hashes are different, we can quickly conclude "inequal". If the pointers are equal, we can quickly conclude "equal". (Many linkers should be able to deduplicate the data the pointers point to, making the "equal" optimization even work cross-crate in many cases.) The same approach could easily be used without a full type name, by storing a pair of a low-quality hash for quick "inequal" checks and a high-equality hash behind the pointer to provide soundness.

Relying on the linker to deduplicate is unlikely to work since not all platforms have linkers that can do that (in particular for dynamic linking). C++ has a similar problem to solve and, at least on Windows, seems to do something like the first of these options: compare pointers, and fall back to comparing strings.

Therefore, the next step here seems to be for the compiler team to decide whether to use a hash function or to include the type name in the binary. The latter has an existing implementation, but there were concerns about leaking type names (and the paths leading to those types) in the binary.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the TypeId and downcast behavior described here, then read PR #95845 and the linked discussion on the current type_id implementation. Compare the proposed stronger-hash and type-name approaches with their soundness and binary-leakage concerns. Done means the compiler team has selected and implemented a sound scheme with appropriate tests.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
compilers
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.