Skip to content

Commit 6b63721

Browse files
committed
fix: runtime hardening — memory leaks, overflow protection, bounds validation
DynamicBox memory leaks: - io_concat_dynamic: add drop_zrtl_string dropper for allocated result strings - io_string_to_dynamic: copy input string so DynamicBox owns its data with dropper - format_dynamic_box: free display_fn-returned ZRTL strings after use (leaked on every tensor print) ZyntaxArray overflow protection: - with_capacity: checked_mul/checked_add for allocation size, cap at i32::MAX - grow: same overflow protection for reallocation path - Use handle_alloc_error instead of panic on null alloc Bounds validation in format_dynamic_box: - Array display: validate len <= capacity, null data check, cap sanity limit - Tuple display: alignment validation, element count sanity check, null check Tests: 9 new tests for DynamicBox dropper, null safety, type formatting
1 parent 7d27b33 commit 6b63721

3 files changed

Lines changed: 275 additions & 24 deletions

File tree

crates/zyntax_embed/src/array.rs

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,16 +39,24 @@ pub struct ZyntaxArray<T: Copy> {
3939

4040
impl<T: Copy> ZyntaxArray<T> {
4141
/// Create a new empty array with the given capacity
42+
///
43+
/// # Panics
44+
/// Panics if the allocation size overflows or if allocation fails.
4245
pub fn with_capacity(capacity: usize) -> Self {
43-
let capacity = capacity.max(1) as i32;
44-
let total_size = ARRAY_HEADER_SIZE + (capacity as usize) * std::mem::size_of::<T>();
46+
let capacity = capacity.max(1).min(i32::MAX as usize) as i32;
47+
let elem_bytes = (capacity as usize)
48+
.checked_mul(std::mem::size_of::<T>())
49+
.expect("ZyntaxArray: capacity * element size overflows");
50+
let total_size = ARRAY_HEADER_SIZE
51+
.checked_add(elem_bytes)
52+
.expect("ZyntaxArray: total allocation size overflows");
4553

4654
unsafe {
4755
let layout = std::alloc::Layout::from_size_align(total_size, 4).unwrap();
4856
let ptr = std::alloc::alloc(layout) as *mut i32;
4957

5058
if ptr.is_null() {
51-
panic!("Failed to allocate ZyntaxArray");
59+
std::alloc::handle_alloc_error(layout);
5260
}
5361

5462
// Write header
@@ -239,19 +247,25 @@ impl<T: Copy> ZyntaxArray<T> {
239247
/// Grow the array capacity (doubles it)
240248
fn grow(&mut self) {
241249
let old_cap = self.capacity();
242-
let new_cap = (old_cap * 2).max(8);
250+
let new_cap = (old_cap * 2).max(8).min(i32::MAX as usize);
243251
let len = self.len();
244252

245253
let old_size = ARRAY_HEADER_SIZE + old_cap * std::mem::size_of::<T>();
246-
let new_size = ARRAY_HEADER_SIZE + new_cap * std::mem::size_of::<T>();
254+
let new_elem_bytes = new_cap
255+
.checked_mul(std::mem::size_of::<T>())
256+
.expect("ZyntaxArray grow: capacity * element size overflows");
257+
let new_size = ARRAY_HEADER_SIZE
258+
.checked_add(new_elem_bytes)
259+
.expect("ZyntaxArray grow: total allocation size overflows");
247260

248261
unsafe {
249262
let old_layout = std::alloc::Layout::from_size_align_unchecked(old_size, 4);
250263
let new_ptr =
251264
std::alloc::realloc(self.ptr.as_ptr() as *mut u8, old_layout, new_size) as *mut i32;
252265

253266
if new_ptr.is_null() {
254-
panic!("Failed to grow ZyntaxArray");
267+
let new_layout = std::alloc::Layout::from_size_align_unchecked(new_size, 4);
268+
std::alloc::handle_alloc_error(new_layout);
255269
}
256270

257271
*new_ptr = new_cap as i32;

plugins/zrtl_io/src/lib.rs

Lines changed: 249 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,20 @@ use zrtl::{
3232
string_length, string_data, string_new, string_free,
3333
};
3434

35+
// ============================================================================
36+
// DynamicBox Dropper Functions
37+
// ============================================================================
38+
39+
/// Dropper for DynamicBox values whose `data` field is a ZRTL string pointer.
40+
/// Frees the string using the ZRTL allocation layout (header + data bytes).
41+
extern "C" fn drop_zrtl_string(ptr: *mut u8) {
42+
if ptr.is_null() {
43+
return;
44+
}
45+
unsafe { string_free(ptr as StringPtr) }
46+
}
47+
48+
3549
// ============================================================================
3650
// String I/O Functions (using ZRTL string format)
3751
// ============================================================================
@@ -456,7 +470,15 @@ unsafe fn format_dynamic_box(value: &zrtl::DynamicBox, output: &mut String) {
456470
return;
457471
}
458472

473+
let capacity = zrtl::array_capacity(arr_ptr) as usize;
459474
let len = zrtl::array_length(arr_ptr) as usize;
475+
476+
// Validate: length must not exceed capacity
477+
if len > capacity || capacity > (1 << 28) {
478+
let _ = write!(output, "<array len={} cap={} (invalid)>", len, capacity);
479+
return;
480+
}
481+
460482
output.push('[');
461483

462484
// Determine element type from size (heuristic)
@@ -469,7 +491,14 @@ unsafe fn format_dynamic_box(value: &zrtl::DynamicBox, output: &mut String) {
469491

470492
// Try to print elements based on common element sizes
471493
let data_ptr = zrtl::array_data::<u8>(arr_ptr);
472-
for i in 0..len.min(10) { // Limit to 10 elements for display
494+
if data_ptr.is_null() {
495+
output.push_str("<null data>");
496+
output.push(']');
497+
return;
498+
}
499+
500+
let display_len = len.min(10);
501+
for i in 0..display_len {
473502
if i > 0 { output.push_str(", "); }
474503

475504
match elem_size {
@@ -515,10 +544,24 @@ unsafe fn format_dynamic_box(value: &zrtl::DynamicBox, output: &mut String) {
515544
TypeCategory::Tuple => {
516545
// Tuple is stored as consecutive DynamicBox values
517546
// Size tells us total bytes, each DynamicBox is 32 bytes (on 64-bit)
518-
let num_elements = value.size as usize / std::mem::size_of::<zrtl::DynamicBox>();
547+
let box_size = std::mem::size_of::<zrtl::DynamicBox>();
548+
if box_size == 0 || (value.size as usize) % box_size != 0 {
549+
let _ = write!(output, "<tuple {} bytes (misaligned)>", value.size);
550+
return;
551+
}
552+
let num_elements = value.size as usize / box_size;
553+
if num_elements > 256 {
554+
let _ = write!(output, "<tuple {} elements (suspicious)>", num_elements);
555+
return;
556+
}
519557
output.push('(');
520558
let elements = value.data as *const zrtl::DynamicBox;
521-
for i in 0..num_elements.min(10) {
559+
if elements.is_null() {
560+
output.push_str("<null>)");
561+
return;
562+
}
563+
let display_count = num_elements.min(10);
564+
for i in 0..display_count {
522565
if i > 0 { output.push_str(", "); }
523566
let elem = &*elements.add(i);
524567
format_dynamic_box(elem, output);
@@ -607,15 +650,15 @@ unsafe fn format_dynamic_box(value: &zrtl::DynamicBox, output: &mut String) {
607650
// Call the display function to format the value
608651
let formatted_ptr = display_fn(value.data as *const u8);
609652
if !formatted_ptr.is_null() {
610-
// Display function returned a ZRTL string pointer
611-
// Cast from *const u8 to StringConstPtr (*const i32)
653+
// Display function returned a heap-allocated ZRTL string pointer
612654
let str_ptr = formatted_ptr as StringConstPtr;
613655
if let Some(formatted_str) = zrtl::string_as_str(str_ptr) {
614656
output.push_str(formatted_str);
615657
} else {
616658
output.push_str("<opaque (display invalid utf8)>");
617659
}
618-
// The string is owned by the display function, don't free it here
660+
// Free the display function's returned string (it was heap-allocated)
661+
string_free(formatted_ptr as StringPtr);
619662
} else {
620663
// Display function returned null - fall back to hex dump
621664
output.push_str("<opaque (display failed)>");
@@ -822,8 +865,12 @@ pub unsafe extern "C" fn io_println_array_f64(arr: zrtl::ArrayConstPtr) {
822865
// String Concatenation
823866
// ============================================================================
824867

825-
/// Convert a string pointer to a DynamicBox
826-
/// Used by f-string desugaring to make string results compatible with println_dynamic
868+
/// Convert a string pointer to a DynamicBox (borrows the string, caller manages lifetime)
869+
///
870+
/// # Safety
871+
/// - `s` must be a valid ZRTL string pointer or null
872+
/// - The string must remain valid for the lifetime of the returned DynamicBox
873+
/// - The caller must free the returned DynamicBox with `Box::from_raw`
827874
#[no_mangle]
828875
pub unsafe extern "C" fn io_string_to_dynamic(s: StringConstPtr) -> *mut zrtl::DynamicBox {
829876
use zrtl::{DynamicBox, TypeTag};
@@ -832,15 +879,15 @@ pub unsafe extern "C" fn io_string_to_dynamic(s: StringConstPtr) -> *mut zrtl::D
832879
return Box::into_raw(Box::new(DynamicBox::null()));
833880
}
834881

835-
// The DynamicBox for String category expects data to be a StringConstPtr
836-
// (pointer to ZRTL string format: [i32 length][utf8 bytes])
837-
// We just wrap the existing pointer - caller is responsible for memory
882+
// Copy the string so the DynamicBox owns its data and can free it via dropper
883+
let copy = zrtl::string_copy(s);
884+
838885
Box::into_raw(Box::new(DynamicBox {
839886
tag: TypeTag::STRING,
840887
size: std::mem::size_of::<*const u8>() as u32,
841-
data: s as *mut u8,
842-
dropper: None, // Don't free the string, caller manages it
843-
display_fn: None, // Use default string formatting
888+
data: copy as *mut u8,
889+
dropper: Some(drop_zrtl_string),
890+
display_fn: None,
844891
}))
845892
}
846893

@@ -933,7 +980,7 @@ pub unsafe extern "C" fn io_concat_dynamic(a: *const zrtl::DynamicBox, b: *const
933980
tag: TypeTag::STRING,
934981
size: std::mem::size_of::<*const u8>() as u32,
935982
data: str_ptr as *mut u8,
936-
dropper: None, // TODO: Add dropper to free the string
983+
dropper: Some(drop_zrtl_string),
937984
display_fn: None,
938985
}))
939986
}
@@ -1118,4 +1165,191 @@ mod tests {
11181165
string_free(formatted);
11191166
}
11201167
}
1168+
1169+
#[test]
1170+
fn test_string_to_dynamic_copies_string() {
1171+
// Verify io_string_to_dynamic creates an owned copy
1172+
let original = string_new("hello");
1173+
unsafe {
1174+
let boxed_ptr = io_string_to_dynamic(original);
1175+
assert!(!boxed_ptr.is_null());
1176+
1177+
let boxed = &*boxed_ptr;
1178+
assert_eq!(boxed.tag.category(), TypeCategory::String);
1179+
assert!(boxed.dropper.is_some(), "DynamicBox should have a dropper");
1180+
1181+
// The data pointer should be different from the original (it's a copy)
1182+
assert_ne!(boxed.data as *const u8, original as *const u8 as *const u8);
1183+
1184+
// Verify the copied string content
1185+
let str_ptr = boxed.data as StringConstPtr;
1186+
assert_eq!(string_as_str(str_ptr), Some("hello"));
1187+
1188+
// Clean up: free the DynamicBox's string via dropper, the Box, and the original
1189+
if let Some(dropper) = boxed.dropper {
1190+
dropper(boxed.data);
1191+
}
1192+
let _ = Box::from_raw(boxed_ptr);
1193+
string_free(original);
1194+
}
1195+
}
1196+
1197+
#[test]
1198+
fn test_string_to_dynamic_null_input() {
1199+
unsafe {
1200+
let boxed_ptr = io_string_to_dynamic(std::ptr::null());
1201+
assert!(!boxed_ptr.is_null());
1202+
let boxed = &*boxed_ptr;
1203+
assert!(boxed.data.is_null(), "null input should produce null DynamicBox");
1204+
let _ = Box::from_raw(boxed_ptr);
1205+
}
1206+
}
1207+
1208+
#[test]
1209+
fn test_concat_dynamic_has_dropper() {
1210+
// Verify io_concat_dynamic creates DynamicBox with string dropper
1211+
let s1 = string_new("hello ");
1212+
let s2 = string_new("world");
1213+
1214+
unsafe {
1215+
let box1 = io_string_to_dynamic(s1);
1216+
let box2 = io_string_to_dynamic(s2);
1217+
1218+
let result = io_concat_dynamic(box1, box2);
1219+
assert!(!result.is_null());
1220+
1221+
let result_box = &*result;
1222+
assert_eq!(result_box.tag.category(), TypeCategory::String);
1223+
assert!(result_box.dropper.is_some(), "concat result should have a string dropper");
1224+
1225+
// Verify content
1226+
let str_ptr = result_box.data as StringConstPtr;
1227+
assert_eq!(string_as_str(str_ptr), Some("hello world"));
1228+
1229+
// Clean up (call dropper for the string, then free the Box)
1230+
if let Some(dropper) = result_box.dropper {
1231+
dropper(result_box.data);
1232+
}
1233+
let _ = Box::from_raw(result);
1234+
1235+
// Clean up inputs
1236+
let b1 = &*box1;
1237+
if let Some(dropper) = b1.dropper { dropper(b1.data); }
1238+
let _ = Box::from_raw(box1);
1239+
let b2 = &*box2;
1240+
if let Some(dropper) = b2.dropper { dropper(b2.data); }
1241+
let _ = Box::from_raw(box2);
1242+
1243+
string_free(s1);
1244+
string_free(s2);
1245+
}
1246+
}
1247+
1248+
#[test]
1249+
fn test_format_dynamic_null_safety() {
1250+
unsafe {
1251+
// Null pointer should return "null"
1252+
let formatted = io_format_dynamic(std::ptr::null());
1253+
assert!(!formatted.is_null());
1254+
assert_eq!(string_as_str(formatted), Some("null"));
1255+
string_free(formatted);
1256+
}
1257+
}
1258+
1259+
#[test]
1260+
fn test_format_dynamic_null_data_box() {
1261+
// DynamicBox with null data field
1262+
let boxed = zrtl::DynamicBox::null();
1263+
unsafe {
1264+
let formatted = io_format_dynamic(&boxed as *const zrtl::DynamicBox);
1265+
assert!(!formatted.is_null());
1266+
assert_eq!(string_as_str(formatted), Some("null"));
1267+
string_free(formatted);
1268+
}
1269+
}
1270+
1271+
#[test]
1272+
fn test_format_dynamic_int_types() {
1273+
// Test various integer sizes
1274+
let val: i32 = -42;
1275+
let boxed = zrtl::DynamicBox {
1276+
tag: TypeTag::new(TypeCategory::Int, 0, TypeFlags::NONE),
1277+
size: 4,
1278+
data: &val as *const i32 as *mut u8,
1279+
dropper: None,
1280+
display_fn: None,
1281+
};
1282+
unsafe {
1283+
let formatted = io_format_dynamic(&boxed as *const zrtl::DynamicBox);
1284+
assert_eq!(string_as_str(formatted), Some("-42"));
1285+
string_free(formatted);
1286+
}
1287+
1288+
let val64: i64 = 9999999;
1289+
let boxed64 = zrtl::DynamicBox {
1290+
tag: TypeTag::new(TypeCategory::Int, 0, TypeFlags::NONE),
1291+
size: 8,
1292+
data: &val64 as *const i64 as *mut u8,
1293+
dropper: None,
1294+
display_fn: None,
1295+
};
1296+
unsafe {
1297+
let formatted = io_format_dynamic(&boxed64 as *const zrtl::DynamicBox);
1298+
assert_eq!(string_as_str(formatted), Some("9999999"));
1299+
string_free(formatted);
1300+
}
1301+
}
1302+
1303+
#[test]
1304+
fn test_format_dynamic_float_types() {
1305+
let val: f64 = 3.14;
1306+
let boxed = zrtl::DynamicBox {
1307+
tag: TypeTag::new(TypeCategory::Float, 0, TypeFlags::NONE),
1308+
size: 8,
1309+
data: &val as *const f64 as *mut u8,
1310+
dropper: None,
1311+
display_fn: None,
1312+
};
1313+
unsafe {
1314+
let formatted = io_format_dynamic(&boxed as *const zrtl::DynamicBox);
1315+
let s = string_as_str(formatted).unwrap_or_default();
1316+
assert!(s.starts_with("3.14"), "expected 3.14..., got: {}", s);
1317+
string_free(formatted);
1318+
}
1319+
}
1320+
1321+
#[test]
1322+
fn test_format_dynamic_bool() {
1323+
let val: u8 = 1;
1324+
let boxed = zrtl::DynamicBox {
1325+
tag: TypeTag::new(TypeCategory::Bool, 0, TypeFlags::NONE),
1326+
size: 1,
1327+
data: &val as *const u8 as *mut u8,
1328+
dropper: None,
1329+
display_fn: None,
1330+
};
1331+
unsafe {
1332+
let formatted = io_format_dynamic(&boxed as *const zrtl::DynamicBox);
1333+
assert_eq!(string_as_str(formatted), Some("true"));
1334+
string_free(formatted);
1335+
}
1336+
}
1337+
1338+
#[test]
1339+
fn test_format_dynamic_string() {
1340+
let s = string_new("test string");
1341+
let boxed = zrtl::DynamicBox {
1342+
tag: TypeTag::STRING,
1343+
size: std::mem::size_of::<*const u8>() as u32,
1344+
data: s as *mut u8,
1345+
dropper: None,
1346+
display_fn: None,
1347+
};
1348+
unsafe {
1349+
let formatted = io_format_dynamic(&boxed as *const zrtl::DynamicBox);
1350+
assert_eq!(string_as_str(formatted), Some("test string"));
1351+
string_free(formatted);
1352+
string_free(s);
1353+
}
1354+
}
11211355
}

0 commit comments

Comments
 (0)