What the developer wrote
Authored by Niklas Dusenlund
Fix some memory handling issues
What changed, and why it matters
This commit fixes two memory-handling bugs in the BitBox02 hardware wallet's Rust LVGL UI bindings. First, line widgets were storing point data in dynamically allocated memory attached to the LVGL object and freeing it when the object was deleted; if the same points were shared or the attachment logic failed, this could lead to crashes or memory corruption. The fix requires callers to pass only statically allocated, never-freed point slices. Second, style-transition property lists were accepted without checking for a required terminator marker; an unterminated list could cause LVGL to read past the end of the array, leading to crashes or undefined behavior. The commit adds validation that panics at construction time if the terminator is missing.
Review all callers of `LvLine::set_points` to ensure they now supply `&'static [LvPointPrecise]` slices; any caller still passing a Vec or non-static slice will fail to compile. Confirm that existing static point data used in the firmware meets the new lifetime contract. For `LvStyleTransition`, verify all existing call sites already terminate their property lists with `prop::INV`; the new panic will catch violations at runtime during construction. Consider whether the panic in `LvStyleTransition::new` is acceptable for a hardware wallet UI or whether it should return a `Result` to avoid denial-of-service.
Security signals we found
Memory lifetime mismatch between Rust-owned Vec and C object lifetime
Potential use-after-free / double-free in object-attached heap storage
Missing terminator validation on C-style sentinel array passed to LVGL
Out-of-bounds read risk in style transition property list
API changed to require 'static lifetime for externally-retained buffers
Evidence from the diff
The patch changes LvLine::set_points from taking an owned Vec<LvPointPrecise> (which was attached to the LVGL object and freed on object deletion via util::attach_to_object) to taking a &'static [LvPointPrecise]. This removes a use-after-free/double-free risk if the attached Vec outlived its intended lifetime or if callers relied on the previous heap allocation. It also removes the LvLineError::PointAttachmentFailed error variant. Separately, LvStyleTransition::new now validates that the props slice is non-empty and terminated by prop::INV; previously it blindly passed the pointer to LVGL, which iterates the list until INV, risking out-of-bounds reads. Unit tests are added for both behaviors.
Changed components
src/rust/bitbox-lvgl/src/widgets/line.rssrc/rust/bitbox-lvgl/src/style.rsLvLine widget point storageLvStyleTransition descriptor constructionInspect captured patch +84 / −26
diff --git a/src/rust/bitbox-lvgl/src/lib.rs b/src/rust/bitbox-lvgl/src/lib.rs
index d42d4a7..e32b410 100644
--- a/src/rust/bitbox-lvgl/src/lib.rs
+++ b/src/rust/bitbox-lvgl/src/lib.rs
@@ -82,7 +82,7 @@ pub use widgets::class;
pub use widgets::image::ImageExt;
pub use widgets::keyboard::{KeyboardExt, LvKeyboard, LvKeyboardMapEntry, keyboard_def_event_cb};
pub use widgets::label::{LabelExt, LvLabel, LvLabelTextError};
-pub use widgets::line::{LvLine, LvLineError};
+pub use widgets::line::LvLine;
pub use widgets::obj;
pub use widgets::obj::LvObj;
pub use widgets::obj::{LvHandle, LvTypeError, ObjExt};
diff --git a/src/rust/bitbox-lvgl/src/style.rs b/src/rust/bitbox-lvgl/src/style.rs
index c7328df..e90565f 100644
--- a/src/rust/bitbox-lvgl/src/style.rs
+++ b/src/rust/bitbox-lvgl/src/style.rs
@@ -29,6 +29,10 @@ unsafe impl Sync for LvStyleTransition {}
impl LvStyleTransition {
/// `props` must be a `'static` slice terminated by [`prop::INV`].
pub const fn new(props: &'static [u8], time_ms: u32, delay_ms: u32) -> Self {
+ if props.is_empty() || props[props.len() - 1] != prop::INV {
+ panic!("style transition props must be terminated by prop::INV");
+ }
+
Self(ffi::lv_style_transition_dsc_t {
props: props.as_ptr(),
user_data: core::ptr::null_mut(),
@@ -44,3 +48,31 @@ impl LvStyleTransition {
&self.0
}
}
+
+#[cfg(test)]
+mod tests {
+ use super::*;
+
+ const VALID_PROPS: &[u8] = &[prop::BG_OPA, prop::BG_COLOR, prop::INV];
+ const VALID_TRANSITION: LvStyleTransition = LvStyleTransition::new(VALID_PROPS, 1, 2);
+
+ #[test]
+ fn test_style_transition_new() {
+ let dsc = VALID_TRANSITION.as_dsc();
+ assert_eq!(dsc.props, VALID_PROPS.as_ptr());
+ assert_eq!(dsc.time, 1);
+ assert_eq!(dsc.delay, 2);
+ }
+
+ #[test]
+ #[should_panic]
+ fn test_style_transition_new_rejects_missing_terminator() {
+ let _ = LvStyleTransition::new(&[prop::BG_OPA, prop::BG_COLOR], 1, 0);
+ }
+
+ #[test]
+ #[should_panic]
+ fn test_style_transition_new_rejects_empty_props() {
+ let _ = LvStyleTransition::new(&[], 1, 0);
+ }
+}
diff --git a/src/rust/bitbox-lvgl/src/widgets/line.rs b/src/rust/bitbox-lvgl/src/widgets/line.rs
index 4eea4bf..d67cf1d 100644
--- a/src/rust/bitbox-lvgl/src/widgets/line.rs
+++ b/src/rust/bitbox-lvgl/src/widgets/line.rs
@@ -1,18 +1,11 @@
// SPDX-License-Identifier: Apache-2.0
-use alloc::vec::Vec;
use core::ptr::NonNull;
-use super::util;
use crate::{LvHandle, LvObj, LvPointPrecise, class, ffi};
pub type LvLine = LvHandle<class::LineTag>;
-#[derive(Clone, Copy, Debug, PartialEq, Eq)]
-pub enum LvLineError {
- PointAttachmentFailed,
-}
-
impl LvHandle<class::LineTag> {
pub fn new<P: class::LvClass>(parent: &LvHandle<P>) -> Option<Self> {
NonNull::new(unsafe { ffi::lv_line_create(parent.as_ptr()) }).map(LvHandle::from_ptr)
@@ -21,16 +14,11 @@ impl LvHandle<class::LineTag> {
/// Sets the polyline points.
///
/// LVGL only retains the pointer to the points (it does not copy them), so the points are
- /// stored alongside the object and freed together with it. The line's color, width and rounded
- /// caps are controlled with the `set_style_line_*` methods from [`crate::ObjExt`].
- pub fn set_points(&self, points: Vec<LvPointPrecise>) -> Result<(), LvLineError> {
+ /// must be statically allocated. The line's color, width and rounded caps are controlled with
+ /// the `set_style_line_*` methods from [`crate::ObjExt`].
+ pub fn set_points(&self, points: &'static [LvPointPrecise]) {
let point_num = points.len() as u32;
- let attachment =
- util::attach_to_object(self, points).map_err(|_| LvLineError::PointAttachmentFailed)?;
- unsafe {
- ffi::lv_line_set_points(self.as_ptr(), (*attachment.as_ptr()).as_ptr(), point_num)
- }
- Ok(())
+ unsafe { ffi::lv_line_set_points(self.as_ptr(), points.as_ptr(), point_num) }
}
pub fn to_obj(self) -> LvObj {
@@ -43,21 +31,59 @@ mod tests {
use super::*;
use crate::ObjExt;
+ static POINTS: &[LvPointPrecise] = &[
+ LvPointPrecise { x: 0, y: 0 },
+ LvPointPrecise { x: 10, y: 20 },
+ LvPointPrecise { x: 30, y: 5 },
+ ];
+
+ static POINTS_REPLACEMENT: &[LvPointPrecise] = &[
+ LvPointPrecise { x: 1, y: 2 },
+ LvPointPrecise { x: 3, y: 4 },
+ LvPointPrecise { x: 5, y: 6 },
+ ];
+
+ #[test]
+ fn test_line_set_points_uses_static_points() {
+ let _lock = crate::test_util::lock_and_init();
+
+ let display = crate::LvDisplay::new(64, 64).unwrap();
+ let screen = display.screen_active().unwrap();
+ let line = LvLine::new(&screen).unwrap();
+ line.set_points(POINTS);
+
+ assert_eq!(
+ unsafe { ffi::lv_line_get_points(line.as_ptr()) },
+ POINTS.as_ptr()
+ );
+ assert_eq!(unsafe { ffi::lv_line_get_point_count(line.as_ptr()) }, 3);
+
+ // Deleting the object must not free the statically allocated points.
+ unsafe { line.delete() };
+ }
+
#[test]
- fn test_line_set_points_keeps_points_alive() {
+ fn test_line_set_points_replaces_points() {
let _lock = crate::test_util::lock_and_init();
let display = crate::LvDisplay::new(64, 64).unwrap();
let screen = display.screen_active().unwrap();
let line = LvLine::new(&screen).unwrap();
- line.set_points(alloc::vec![
- LvPointPrecise { x: 0, y: 0 },
- LvPointPrecise { x: 10, y: 20 },
- LvPointPrecise { x: 30, y: 5 },
- ])
- .unwrap();
-
- // Deleting the object frees the attached points without leaking or double-freeing.
+ line.set_points(POINTS);
+ let event_count = unsafe { ffi::lv_obj_get_event_count(line.as_ptr()) };
+
+ line.set_points(POINTS_REPLACEMENT);
+
+ assert_eq!(
+ unsafe { ffi::lv_obj_get_event_count(line.as_ptr()) },
+ event_count
+ );
+ assert_eq!(
+ unsafe { ffi::lv_line_get_points(line.as_ptr()) },
+ POINTS_REPLACEMENT.as_ptr()
+ );
+ assert_eq!(unsafe { ffi::lv_line_get_point_count(line.as_ptr()) }, 3);
+
unsafe { line.delete() };
}
}
Why this scored 59/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.