signalapp / libsignal

Home to the Signal Protocol as well as other cryptographic primitives which make Signal possible.
GNU Affero General Public License v3.0
3.06k stars 362 forks source link

poksho: an impossibly contrived overflow can theoretically happen #480

Open jpdoyle opened 1 year ago

jpdoyle commented 1 year ago

The while i * HASHLEN < outlen pattern in squeeze_and_ratchet for both ShoSha256 and ShoHmacSha256 can overflow if given an extremely large outlen, leading to a panic in debug mode or an infinite memory-eating loop in release mode. A tweaked implementation using u32 and a test case that trips the overflow is available at https://github.com/jpdoyle/libsignal/blob/fb6a53de6c7b26863d1feab1d83c62291ac1be0d/rust/poksho/src/shosha256.rs#L131

So far as I can tell this code path is completely unreachable in any practical scenario, and is at worst able to cause denial-of-service. On 64-bit systems I think you use more RAM than has ever been manufactured (and certainly more than you can fit in most processors' physical address spaces) before it triggers.

The only case I can imagine is a system with a 32-bit usize which will still let you allocate a (2^32 - 63)-length Vec (unlikely unless sizeof(void*) != sizeof(size_t)), plus an input to squeeze_and_ratchet where ((outlen/HASHLEN)+1)*HASHLEN >= (1<<32).

I initially thought that if you have a poksho statement with 67108863 scalar parameters and all these other things line up, you can potentially trigger the overflow via https://github.com/signalapp/libsignal/blob/aa5f7c2b14315e94fa54fbdbda02b3938b218925/rust/poksho/src/statement.rs#L206 -- but it turns out that by including the encoded statement into the transcript, to_bytes gets called and that will fail an assert if there are more than 256 scalars.