Fix safety issues in icu_provider_baked (#6055)

Both of these APIs are unsound: Data has public fields, and the attributes_prefix_match case can index out of bounds.
unicode-org · Feb 3, 2025 · d26e369 · d26e369
1 parent 1b71a22
commit d26e369
Show file tree

Hide file tree

Showing 120 changed files with 617 additions and 292 deletions.
diff --git a/provider/baked/src/binary_search.rs b/provider/baked/src/binary_search.rs
@@ -127,12 +127,14 @@ impl<K: BinarySearchKey, M: DataMarker> super::DataStore<M> for Data<K, M> {
         self.0
             .binary_search_by(|&(k, _)| K::cmp(k, id))
             .or_else(|e| {
-                if attributes_prefix_match {
+                if attributes_prefix_match && e <= self.0.len() {
                     Ok(e)
                 } else {
                     Err(e)
                 }
             })
+            // Safety: binary_search returns in-bounds indices when returning Ok.
+            // The err case in `or_else` above only returns in-bounds Ok values
             .map(|i| unsafe { self.0.get_unchecked(i) }.1)
             .ok()
     }

diff --git a/provider/baked/src/zerotrie.rs b/provider/baked/src/zerotrie.rs
@@ -8,6 +8,7 @@
 const ID_SEPARATOR: u8 = 0x1E;
 
 use icu_provider::prelude::*;
+pub use icu_provider::DynamicDataMarker;
 pub use zerotrie::ZeroTrieSimpleAscii;
 
 #[cfg(feature = "export")]
@@ -22,6 +23,8 @@ pub(crate) fn bake(
 
     let bakes = bakes_to_ids.iter().map(|(bake, _)| bake);
 
+    // Safety invariant upheld: the only values being added to the trie are `baked_index`
+    // values, which come from `bakes`
     let trie = ZeroTrieSimpleAscii::from_iter(bakes_to_ids.iter().enumerate().flat_map(
         |(bake_index, (_, ids))| {
             ids.iter().map(move |id| {
@@ -39,19 +42,38 @@ pub(crate) fn bake(
 
     (
         quote! {
-            icu_provider_baked::zerotrie::Data<#marker_bake> = icu_provider_baked::zerotrie::Data {
-                trie: icu_provider_baked:: #baked_trie,
-                values: &[#(#bakes,)*],
+            // Safety invariant upheld: see above
+            icu_provider_baked::zerotrie::Data<#marker_bake> = {
+                const TRIE: icu_provider_baked::zerotrie::ZeroTrieSimpleAscii<&'static [u8]> = icu_provider_baked:: #baked_trie;
+                const VALUES: &'static [<#marker_bake as icu_provider_baked::zerotrie::DynamicDataMarker>::DataStruct] = &[#(#bakes,)*];
+                unsafe {
+                    icu_provider_baked::zerotrie::Data::from_trie_and_values_unchecked(TRIE, VALUES)
+                }
             }
+
         },
         core::mem::size_of::<Data<icu_provider::hello_world::HelloWorldV1Marker>>()
             + trie.as_borrowed_slice().borrows_size(),
     )
 }
 
 pub struct Data<M: DataMarker> {
-    pub trie: ZeroTrieSimpleAscii<&'static [u8]>,
-    pub values: &'static [M::DataStruct],
+    // Unsafe invariant: actual values contained MUST be valid indices into `values`
+    trie: ZeroTrieSimpleAscii<&'static [u8]>,
+    values: &'static [M::DataStruct],
+}
+
+impl<M: DataMarker> Data<M> {
+    /// Construct from a trie and values
+    ///
+    /// # Safety
+    /// The actual values contained in the trie must be valid indices into `values`
+    pub const unsafe fn from_trie_and_values_unchecked(
+        trie: ZeroTrieSimpleAscii<&'static [u8]>,
+        values: &'static [M::DataStruct],
+    ) -> Self {
+        Self { trie, values }
+    }
 }
 
 impl<M: DataMarker> super::DataStore<M> for Data<M> {
@@ -77,6 +99,7 @@ impl<M: DataMarker> super::DataStore<M> for Data<M> {
         } else {
             cursor.take_value()
         }
+        // Safety: Allowed since `i` came from the trie and the field safety invariant
         .map(|i| unsafe { self.values.get_unchecked(i) })
     }
 

diff --git a/provider/baked/tests/data/hello_world_v1_marker.rs.data b/provider/baked/tests/data/hello_world_v1_marker.rs.data
@@ -16,7 +16,11 @@ macro_rules! __impl_hello_world_v1_marker {
         const _: () = <$provider>::MUST_USE_MAKE_PROVIDER_MACRO;
         #[clippy::msrv = "1.81"]
         impl $provider {
-            const DATA_HELLO_WORLD_V1_MARKER: icu_provider_baked::zerotrie::Data<icu_provider::hello_world::HelloWorldV1Marker> = icu_provider_baked::zerotrie::Data { trie: icu_provider_baked::zerotrie::ZeroTrieSimpleAscii { store: b"\xCDbcdefijlprsvz\x02\x04\nCIKX[^fpsn\x80s\x81e\x82-AT\x83\xC3lno\x012\x84\x85\xC2\x1E-\treverse\x90\t\xC301G\x0C\x0F\xC201\x06\xC212\x01\x86\x879\x8842\x89B\x8A-u-sd-gbeng\x8B\x8C\xC2ai\x01\x8D\x8Es\x8Fa\x90\0\x1Ereverse\x90\na\x90\x01t\x90\x02\xC2ou\x02\x90\x03\x90\x04r\x90\x05-Latn\x90\x06i\x90\x07h\x90\x08" }, values: &[icu_provider::hello_world::HelloWorldV1 { message: alloc::borrow::Cow::Borrowed("ওহে বিশ\u{9cd}ব") }, icu_provider::hello_world::HelloWorldV1 { message: alloc::borrow::Cow::Borrowed("Ahoj světe") }, icu_provider::hello_world::HelloWorldV1 { message: alloc::borrow::Cow::Borrowed("Hallo Welt") }, icu_provider::hello_world::HelloWorldV1 { message: alloc::borrow::Cow::Borrowed("Servus Welt") }, icu_provider::hello_world::HelloWorldV1 { message: alloc::borrow::Cow::Borrowed("Καλημέρα κόσμε") }, icu_provider::hello_world::HelloWorldV1 { message: alloc::borrow::Cow::Borrowed("Hello World") }, icu_provider::hello_world::HelloWorldV1 { message: alloc::borrow::Cow::Borrowed("Hello from 🗺\u{fe0f}") }, icu_provider::hello_world::HelloWorldV1 { message: alloc::borrow::Cow::Borrowed("Hello from 🌍") }, icu_provider::hello_world::HelloWorldV1 { message: alloc::borrow::Cow::Borrowed("Hello from 🌎") }, icu_provider::hello_world::HelloWorldV1 { message: alloc::borrow::Cow::Borrowed("Hello from 🌏") }, icu_provider::hello_world::HelloWorldV1 { message: alloc::borrow::Cow::Borrowed("Hello from 🇬🇧") }, icu_provider::hello_world::HelloWorldV1 { message: alloc::borrow::Cow::Borrowed("Hello from 🏴\u{e0067}\u{e0062}\u{e0065}\u{e006e}\u{e0067}\u{e007f}") }, icu_provider::hello_world::HelloWorldV1 { message: alloc::borrow::Cow::Borrowed("Saluton, Mondo") }, icu_provider::hello_world::HelloWorldV1 { message: alloc::borrow::Cow::Borrowed("سلام دنیا\u{200e}") }, icu_provider::hello_world::HelloWorldV1 { message: alloc::borrow::Cow::Borrowed("hei maailma") }, icu_provider::hello_world::HelloWorldV1 { message: alloc::borrow::Cow::Borrowed("Halló, heimur") }, icu_provider::hello_world::HelloWorldV1 { message: alloc::borrow::Cow::Borrowed("こんにちは世界") }, icu_provider::hello_world::HelloWorldV1 { message: alloc::borrow::Cow::Borrowed("Ave, munde") }, icu_provider::hello_world::HelloWorldV1 { message: alloc::borrow::Cow::Borrowed("Olá, mundo") }, icu_provider::hello_world::HelloWorldV1 { message: alloc::borrow::Cow::Borrowed("Salut, lume") }, icu_provider::hello_world::HelloWorldV1 { message: alloc::borrow::Cow::Borrowed("Привет, мир") }, icu_provider::hello_world::HelloWorldV1 { message: alloc::borrow::Cow::Borrowed("Поздрав свете") }, icu_provider::hello_world::HelloWorldV1 { message: alloc::borrow::Cow::Borrowed("Pozdrav svete") }, icu_provider::hello_world::HelloWorldV1 { message: alloc::borrow::Cow::Borrowed("Xin chào thế giới") }, icu_provider::hello_world::HelloWorldV1 { message: alloc::borrow::Cow::Borrowed("你好世界") }, icu_provider::hello_world::HelloWorldV1 { message: alloc::borrow::Cow::Borrowed("Olleh Dlrow") }, icu_provider::hello_world::HelloWorldV1 { message: alloc::borrow::Cow::Borrowed("界世はちにんこ") }] };
+            const DATA_HELLO_WORLD_V1_MARKER: icu_provider_baked::zerotrie::Data<icu_provider::hello_world::HelloWorldV1Marker> = {
+                const TRIE: icu_provider_baked::zerotrie::ZeroTrieSimpleAscii<&'static [u8]> = icu_provider_baked::zerotrie::ZeroTrieSimpleAscii { store: b"\xCDbcdefijlprsvz\x02\x04\nCIKX[^fpsn\x80s\x81e\x82-AT\x83\xC3lno\x012\x84\x85\xC2\x1E-\treverse\x90\t\xC301G\x0C\x0F\xC201\x06\xC212\x01\x86\x879\x8842\x89B\x8A-u-sd-gbeng\x8B\x8C\xC2ai\x01\x8D\x8Es\x8Fa\x90\0\x1Ereverse\x90\na\x90\x01t\x90\x02\xC2ou\x02\x90\x03\x90\x04r\x90\x05-Latn\x90\x06i\x90\x07h\x90\x08" };
+                const VALUES: &'static [<icu_provider::hello_world::HelloWorldV1Marker as icu_provider_baked::zerotrie::DynamicDataMarker>::DataStruct] = &[icu_provider::hello_world::HelloWorldV1 { message: alloc::borrow::Cow::Borrowed("ওহে বিশ\u{9cd}ব") }, icu_provider::hello_world::HelloWorldV1 { message: alloc::borrow::Cow::Borrowed("Ahoj světe") }, icu_provider::hello_world::HelloWorldV1 { message: alloc::borrow::Cow::Borrowed("Hallo Welt") }, icu_provider::hello_world::HelloWorldV1 { message: alloc::borrow::Cow::Borrowed("Servus Welt") }, icu_provider::hello_world::HelloWorldV1 { message: alloc::borrow::Cow::Borrowed("Καλημέρα κόσμε") }, icu_provider::hello_world::HelloWorldV1 { message: alloc::borrow::Cow::Borrowed("Hello World") }, icu_provider::hello_world::HelloWorldV1 { message: alloc::borrow::Cow::Borrowed("Hello from 🗺\u{fe0f}") }, icu_provider::hello_world::HelloWorldV1 { message: alloc::borrow::Cow::Borrowed("Hello from 🌍") }, icu_provider::hello_world::HelloWorldV1 { message: alloc::borrow::Cow::Borrowed("Hello from 🌎") }, icu_provider::hello_world::HelloWorldV1 { message: alloc::borrow::Cow::Borrowed("Hello from 🌏") }, icu_provider::hello_world::HelloWorldV1 { message: alloc::borrow::Cow::Borrowed("Hello from 🇬🇧") }, icu_provider::hello_world::HelloWorldV1 { message: alloc::borrow::Cow::Borrowed("Hello from 🏴\u{e0067}\u{e0062}\u{e0065}\u{e006e}\u{e0067}\u{e007f}") }, icu_provider::hello_world::HelloWorldV1 { message: alloc::borrow::Cow::Borrowed("Saluton, Mondo") }, icu_provider::hello_world::HelloWorldV1 { message: alloc::borrow::Cow::Borrowed("سلام دنیا\u{200e}") }, icu_provider::hello_world::HelloWorldV1 { message: alloc::borrow::Cow::Borrowed("hei maailma") }, icu_provider::hello_world::HelloWorldV1 { message: alloc::borrow::Cow::Borrowed("Halló, heimur") }, icu_provider::hello_world::HelloWorldV1 { message: alloc::borrow::Cow::Borrowed("こんにちは世界") }, icu_provider::hello_world::HelloWorldV1 { message: alloc::borrow::Cow::Borrowed("Ave, munde") }, icu_provider::hello_world::HelloWorldV1 { message: alloc::borrow::Cow::Borrowed("Olá, mundo") }, icu_provider::hello_world::HelloWorldV1 { message: alloc::borrow::Cow::Borrowed("Salut, lume") }, icu_provider::hello_world::HelloWorldV1 { message: alloc::borrow::Cow::Borrowed("Привет, мир") }, icu_provider::hello_world::HelloWorldV1 { message: alloc::borrow::Cow::Borrowed("Поздрав свете") }, icu_provider::hello_world::HelloWorldV1 { message: alloc::borrow::Cow::Borrowed("Pozdrav svete") }, icu_provider::hello_world::HelloWorldV1 { message: alloc::borrow::Cow::Borrowed("Xin chào thế giới") }, icu_provider::hello_world::HelloWorldV1 { message: alloc::borrow::Cow::Borrowed("你好世界") }, icu_provider::hello_world::HelloWorldV1 { message: alloc::borrow::Cow::Borrowed("Olleh Dlrow") }, icu_provider::hello_world::HelloWorldV1 { message: alloc::borrow::Cow::Borrowed("界世はちにんこ") }];
+                unsafe { icu_provider_baked::zerotrie::Data::from_trie_and_values_unchecked(TRIE, VALUES) }
+            };
         }
         #[clippy::msrv = "1.81"]
         impl icu_provider::DataProvider<icu_provider::hello_world::HelloWorldV1Marker> for $provider {

diff --git a/provider/data/calendar/data/week_data_v2_marker.rs.data b/provider/data/calendar/data/week_data_v2_marker.rs.data