Skip to content

Commit 230480d

Browse files
bors[bot]gg-yb
andauthored
Merge #228
228: godot-core: bug: fix UB in GodotString::chars_(un)checked r=Bromeon a=gg-yb `slice::from_raw_parts` requires its first argument to be non-null, even when `len == 0` [1], resulting in a crash on the expect in `chars_checked` when Godot returns an empty GodotString as a null pointer and a length of 0. I only checked these two usages, but might be worth checking other usages, too. [1] https://doc.rust-lang.org/std/slice/fn.from_raw_parts.html Co-authored-by: Yannick Bühler <[email protected]>
2 parents d63c0a9 + 6998222 commit 230480d

File tree

2 files changed

+18
-0
lines changed

2 files changed

+18
-0
lines changed

godot-core/src/builtin/string.rs

+10
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,11 @@ impl GodotString {
5555
let len = interface_fn!(string_to_utf32_chars)(s, std::ptr::null_mut(), 0);
5656
let ptr = interface_fn!(string_operator_index_const)(s, 0);
5757

58+
// Even when len == 0, from_raw_parts requires ptr != 0
59+
if ptr.is_null() {
60+
return &[];
61+
}
62+
5863
validate_unicode_scalar_sequence(std::slice::from_raw_parts(ptr, len as usize))
5964
.expect("GodotString::chars_checked: string contains invalid unicode scalar values")
6065
}
@@ -71,6 +76,11 @@ impl GodotString {
7176
let s = self.string_sys();
7277
let len = interface_fn!(string_to_utf32_chars)(s, std::ptr::null_mut(), 0);
7378
let ptr = interface_fn!(string_operator_index_const)(s, 0);
79+
80+
// Even when len == 0, from_raw_parts requires ptr != 0
81+
if ptr.is_null() {
82+
return &[];
83+
}
7484
std::slice::from_raw_parts(ptr as *const char, len as usize)
7585
}
7686
}

itest/rust/src/string_test.rs

+8
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,14 @@ fn string_clone() {
5656
assert_eq!(first, cloned);
5757
}
5858

59+
#[itest]
60+
fn empty_string_chars() {
61+
// Tests regression from #228: Null pointer passed to slice::from_raw_parts
62+
let s = GodotString::new();
63+
assert_eq!(s.chars_checked(), &[]);
64+
assert_eq!(unsafe { s.chars_unchecked() }, &[]);
65+
}
66+
5967
// ----------------------------------------------------------------------------------------------------------------------------------------------
6068

6169
#[itest]

0 commit comments

Comments
 (0)