dalek-cryptography / curve25519-dalek

A pure-Rust implementation of group operations on Ristretto and Curve25519
Other
867 stars 439 forks source link

curve25519-dalek-derive performs unsound hiding of unsafety #563

Open Ralith opened 1 year ago

Ralith commented 1 year ago

Consider a documented example: the simd_avx512ifma::func() function is marked as safe, but will invoke undefined behavior if called when the corresponding target feature is missing.

This could be mitigated through an assert for the presence of the target feature before invoking the inner function (which should optimize out when called from a typical dispatch function), or by marking the function unsafe and documenting the requirements imposed upon callers.

This is not directly a hazard to consumers of curve25519-dalek itself, to whom specialized functions are presumably not directly exposed, but complicates auditing and may present a maintenance hazard.

tarcieri commented 6 months ago

This seems difficult to fix without making breaking changes

Ralith commented 6 months ago

An assert as described above would fix this without a breaking change. Fixing unsoundness is a pretty good reason to break compatibility, though!

burdges commented 6 months ago

I suppose https://github.com/rust-lang/rust/issues/120930 means breakage here anyways, but likely addresses this.