Skip to content

Commit 59fcef4

Browse files
Fix range bound overflow in Vec/Bytes slice and GenRange gen_range for u64 (#1703)
### What - Replace unchecked `+1` / `-1` arithmetic in `Bytes::slice`, `Vec::slice`, and `Prng::gen_range` (u64) range bound conversions with `checked_add` / `checked_sub` using `expect_optimized` - Add `expect_optimized` to the `UnwrapOptimized` trait for `Option` and `Result`, which traps via `wasm32::unreachable()` on Wasm and panics with the message in tests - Add test coverage for `Bytes::slice`, `Vec::slice`, and `Prng::gen_range` boundary conditions including overflow edge cases ### Why When compiled with `overflow-checks = false` (the default for release builds), the bare arithmetic in those functions silently wraps on boundary values like `u32::MAX` or `u64::MAX`. This causes the range passed to the host to differ from the caller's intent — for example, `Bytes::slice(0..=u32::MAX)` wraps to `slice(0..0)` returning empty, and `prng.gen_range((Bound::Excluded(u64::MAX), Bound::Unbounded))` wraps to the full `0..=u64::MAX` range. Contracts using user-controlled or computed range bounds could silently operate on wrong data or generate random numbers from an unintended range. The tooling for Soroban contracts that generates boilerplate for new contracts sets `overflow-checks = true` for the release profile, and docs encourage the same pattern, but this ensures that these overflows are caught even if a developer accidentally turns that off. (cherry picked from commit c2757c6)
1 parent 7870f0d commit 59fcef4

8 files changed

Lines changed: 321 additions & 6 deletions

File tree

soroban-sdk/src/bytes.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -739,11 +739,15 @@ impl Bytes {
739739
pub fn slice(&self, r: impl RangeBounds<u32>) -> Self {
740740
let start_bound = match r.start_bound() {
741741
Bound::Included(s) => *s,
742-
Bound::Excluded(s) => *s + 1,
742+
Bound::Excluded(s) => s
743+
.checked_add(1)
744+
.expect_optimized("attempt to add with overflow"),
743745
Bound::Unbounded => 0,
744746
};
745747
let end_bound = match r.end_bound() {
746-
Bound::Included(s) => *s + 1,
748+
Bound::Included(s) => s
749+
.checked_add(1)
750+
.expect_optimized("attempt to add with overflow"),
747751
Bound::Excluded(s) => *s,
748752
Bound::Unbounded => self.len(),
749753
};

soroban-sdk/src/prng.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -469,12 +469,16 @@ impl GenRange for u64 {
469469
assert_in_contract!(env);
470470
let start_bound = match r.start_bound() {
471471
Bound::Included(b) => *b,
472-
Bound::Excluded(b) => *b + 1,
472+
Bound::Excluded(b) => b
473+
.checked_add(1)
474+
.expect_optimized("attempt to add with overflow"),
473475
Bound::Unbounded => u64::MIN,
474476
};
475477
let end_bound = match r.end_bound() {
476478
Bound::Included(b) => *b,
477-
Bound::Excluded(b) => *b - 1,
479+
Bound::Excluded(b) => b
480+
.checked_sub(1)
481+
.expect_optimized("attempt to subtract with overflow"),
478482
Bound::Unbounded => u64::MAX,
479483
};
480484
internal::Env::prng_u64_in_inclusive_range(env, start_bound, end_bound).unwrap_infallible()

soroban-sdk/src/tests.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ mod address_payload;
55
mod auth;
66
mod bytes_alloc_vec;
77
mod bytes_buffer;
8+
mod bytes_slice;
89
mod cmp_across_env_in_tests;
910
mod contract_add_i32;
1011
mod contract_assert;
@@ -40,7 +41,9 @@ mod env;
4041
mod max_ttl;
4142
mod muxed_address;
4243
mod prng;
44+
mod prng_range;
4345
mod proptest_scval_cmp;
4446
mod proptest_val_cmp;
4547
mod storage_testutils;
4648
mod token_client;
49+
mod vec_slice;
Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
use core::ops::Bound;
2+
3+
use crate::{Bytes, Env};
4+
5+
#[test]
6+
fn test_slice_full_range() {
7+
let env = Env::default();
8+
let bytes = Bytes::from_slice(&env, &[1, 2, 3]);
9+
let sliced = bytes.slice(0..3);
10+
assert_eq!(sliced.len(), 3);
11+
assert_eq!(sliced.get_unchecked(0), 1);
12+
assert_eq!(sliced.get_unchecked(1), 2);
13+
assert_eq!(sliced.get_unchecked(2), 3);
14+
}
15+
16+
#[test]
17+
fn test_slice_middle() {
18+
let env = Env::default();
19+
let bytes = Bytes::from_slice(&env, &[1, 2, 3]);
20+
let sliced = bytes.slice(1..2);
21+
assert_eq!(sliced.len(), 1);
22+
assert_eq!(sliced.get_unchecked(0), 2);
23+
}
24+
25+
// Excluded(u32::MAX) start computes u32::MAX + 1 which overflows.
26+
#[test]
27+
#[should_panic(expected = "attempt to add with overflow")]
28+
fn test_slice_excluded_start_u32_max_overflows_panics_guest_side() {
29+
let env = Env::default();
30+
let bytes = Bytes::from_slice(&env, &[1, 2, 3]);
31+
let _ = bytes.slice((Bound::Excluded(u32::MAX), Bound::Unbounded));
32+
}
33+
34+
// Included(u32::MAX) end computes u32::MAX + 1 which overflows.
35+
#[test]
36+
#[should_panic(expected = "attempt to add with overflow")]
37+
fn test_slice_included_end_u32_max_overflows_panics_guest_side() {
38+
let env = Env::default();
39+
let bytes = Bytes::from_slice(&env, &[1, 2, 3]);
40+
let _ = bytes.slice(0..=u32::MAX);
41+
}
42+
43+
// Both bounds u32::MAX: Excluded start computes u32::MAX + 1 which overflows,
44+
// and Included end computes u32::MAX + 1 which also overflows.
45+
#[test]
46+
#[should_panic(expected = "attempt to add with overflow")]
47+
fn test_slice_both_u32_max_overflows_panics_guest_side() {
48+
let env = Env::default();
49+
let bytes = Bytes::from_slice(&env, &[1, 2, 3]);
50+
let _ = bytes.slice(u32::MAX..=u32::MAX);
51+
}
52+
53+
#[test]
54+
#[should_panic(expected = "HostError: Error(Object, IndexBounds)")]
55+
fn test_slice_out_of_bounds_end_host_side() {
56+
let env = Env::default();
57+
let bytes = Bytes::from_slice(&env, &[1, 2, 3]);
58+
let _ = bytes.slice(0..4);
59+
}
60+
61+
#[test]
62+
#[should_panic(expected = "HostError: Error(Object, IndexBounds)")]
63+
fn test_slice_out_of_bounds_start_host_side() {
64+
let env = Env::default();
65+
let bytes = Bytes::from_slice(&env, &[1, 2, 3]);
66+
let _ = bytes.slice(4..);
67+
}
68+
69+
#[test]
70+
fn test_slice_empty_full_range() {
71+
let env = Env::default();
72+
let bytes = Bytes::new(&env);
73+
let sliced = bytes.slice(..);
74+
assert_eq!(sliced.len(), 0);
75+
}
76+
77+
#[test]
78+
fn test_slice_empty_zero_range() {
79+
let env = Env::default();
80+
let bytes = Bytes::new(&env);
81+
let sliced = bytes.slice(0..0);
82+
assert_eq!(sliced.len(), 0);
83+
}
Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
use core::ops::Bound;
2+
3+
use crate::{self as soroban_sdk, testutils::EnvTestConfig, Env};
4+
use soroban_sdk::contract;
5+
6+
#[contract]
7+
pub struct TestPrngRangeContract;
8+
9+
#[test]
10+
fn test_gen_range_u64_full_range() {
11+
let env = Env::new_with_config(EnvTestConfig {
12+
capture_snapshot_at_drop: false,
13+
});
14+
let id = env.register(TestPrngRangeContract, ());
15+
env.as_contract(&id, || {
16+
let _: u64 = env.prng().gen_range(..);
17+
});
18+
}
19+
20+
#[test]
21+
fn test_gen_range_u64_inclusive() {
22+
let env = Env::new_with_config(EnvTestConfig {
23+
capture_snapshot_at_drop: false,
24+
});
25+
let id = env.register(TestPrngRangeContract, ());
26+
env.as_contract(&id, || {
27+
for _ in 0..100 {
28+
let val: u64 = env.prng().gen_range(5..=10);
29+
assert!(val >= 5 && val <= 10);
30+
}
31+
});
32+
}
33+
34+
#[test]
35+
fn test_gen_range_u64_exclusive_end() {
36+
let env = Env::new_with_config(EnvTestConfig {
37+
capture_snapshot_at_drop: false,
38+
});
39+
let id = env.register(TestPrngRangeContract, ());
40+
env.as_contract(&id, || {
41+
for _ in 0..100 {
42+
let val: u64 = env.prng().gen_range(5..10);
43+
assert!(val >= 5 && val < 10);
44+
}
45+
});
46+
}
47+
48+
#[test]
49+
fn test_gen_range_u64_single_value() {
50+
let env = Env::new_with_config(EnvTestConfig {
51+
capture_snapshot_at_drop: false,
52+
});
53+
let id = env.register(TestPrngRangeContract, ());
54+
env.as_contract(&id, || {
55+
let val: u64 = env.prng().gen_range(7..=7);
56+
assert_eq!(val, 7);
57+
});
58+
}
59+
60+
// Excluded(u64::MAX) start computes u64::MAX + 1 which overflows.
61+
#[test]
62+
#[should_panic(expected = "attempt to add with overflow")]
63+
fn test_gen_range_u64_excluded_start_u64_max_overflows() {
64+
let env = Env::new_with_config(EnvTestConfig {
65+
capture_snapshot_at_drop: false,
66+
});
67+
let id = env.register(TestPrngRangeContract, ());
68+
env.as_contract(&id, || {
69+
let _: u64 = env
70+
.prng()
71+
.gen_range((Bound::Excluded(u64::MAX), Bound::Unbounded));
72+
});
73+
}
74+
75+
#[test]
76+
fn test_gen_range_u64_included_end_u64_max() {
77+
let env = Env::new_with_config(EnvTestConfig {
78+
capture_snapshot_at_drop: false,
79+
});
80+
let id = env.register(TestPrngRangeContract, ());
81+
env.as_contract(&id, || {
82+
let _: u64 = env.prng().gen_range(0u64..=u64::MAX);
83+
});
84+
}
85+
86+
// Excluded(0) end computes 0 - 1 which underflows.
87+
#[should_panic(expected = "attempt to subtract with overflow")]
88+
#[test]
89+
fn test_gen_range_u64_excluded_end_zero_underflows() {
90+
let env = Env::new_with_config(EnvTestConfig {
91+
capture_snapshot_at_drop: false,
92+
});
93+
let id = env.register(TestPrngRangeContract, ());
94+
env.as_contract(&id, || {
95+
let _: u64 = env
96+
.prng()
97+
.gen_range((Bound::<u64>::Unbounded, Bound::Excluded(0)));
98+
});
99+
}
100+
101+
#[test]
102+
fn test_gen_range_u64_both_u64_max() {
103+
let env = Env::new_with_config(EnvTestConfig {
104+
capture_snapshot_at_drop: false,
105+
});
106+
let id = env.register(TestPrngRangeContract, ());
107+
env.as_contract(&id, || {
108+
let val: u64 = env.prng().gen_range(u64::MAX..=u64::MAX);
109+
assert_eq!(val, u64::MAX);
110+
});
111+
}

soroban-sdk/src/tests/vec_slice.rs

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
use core::ops::Bound;
2+
3+
use crate::{vec, Env, Vec};
4+
5+
#[test]
6+
fn test_slice_full_range() {
7+
let env = Env::default();
8+
let v: Vec<u32> = vec![&env, 1, 2, 3];
9+
let sliced = v.slice(0..3);
10+
assert_eq!(sliced.len(), 3);
11+
assert_eq!(sliced.get_unchecked(0), 1);
12+
assert_eq!(sliced.get_unchecked(1), 2);
13+
assert_eq!(sliced.get_unchecked(2), 3);
14+
}
15+
16+
#[test]
17+
fn test_slice_middle() {
18+
let env = Env::default();
19+
let v: Vec<u32> = vec![&env, 1, 2, 3];
20+
let sliced = v.slice(1..2);
21+
assert_eq!(sliced.len(), 1);
22+
assert_eq!(sliced.get_unchecked(0), 2);
23+
}
24+
25+
// Excluded(u32::MAX) start computes u32::MAX + 1 which overflows.
26+
#[test]
27+
#[should_panic(expected = "attempt to add with overflow")]
28+
fn test_slice_excluded_start_u32_max_overflows_panics_guest_side() {
29+
let env = Env::default();
30+
let v: Vec<u32> = vec![&env, 1, 2, 3];
31+
let _ = v.slice((Bound::Excluded(u32::MAX), Bound::Unbounded));
32+
}
33+
34+
// Included(u32::MAX) end computes u32::MAX + 1 which overflows.
35+
#[test]
36+
#[should_panic(expected = "attempt to add with overflow")]
37+
fn test_slice_included_end_u32_max_overflows_panics_guest_side() {
38+
let env = Env::default();
39+
let v: Vec<u32> = vec![&env, 1, 2, 3];
40+
let _ = v.slice(0..=u32::MAX);
41+
}
42+
43+
// Both bounds u32::MAX: Excluded start computes u32::MAX + 1 which overflows,
44+
// and Included end computes u32::MAX + 1 which also overflows.
45+
#[test]
46+
#[should_panic(expected = "attempt to add with overflow")]
47+
fn test_slice_both_u32_max_overflows_panics_guest_side() {
48+
let env = Env::default();
49+
let v: Vec<u32> = vec![&env, 1, 2, 3];
50+
let _ = v.slice(u32::MAX..=u32::MAX);
51+
}
52+
53+
#[test]
54+
#[should_panic(expected = "HostError: Error(Object, IndexBounds)")]
55+
fn test_slice_out_of_bounds_end_host_side() {
56+
let env = Env::default();
57+
let v: Vec<u32> = vec![&env, 1, 2, 3];
58+
let _ = v.slice(0..4);
59+
}
60+
61+
#[test]
62+
#[should_panic(expected = "HostError: Error(Object, IndexBounds)")]
63+
fn test_slice_out_of_bounds_start_host_side() {
64+
let env = Env::default();
65+
let v: Vec<u32> = vec![&env, 1, 2, 3];
66+
let _ = v.slice(4..);
67+
}
68+
69+
#[test]
70+
fn test_slice_empty_full_range() {
71+
let env = Env::default();
72+
let v: Vec<u32> = vec![&env];
73+
let sliced = v.slice(..);
74+
assert_eq!(sliced.len(), 0);
75+
}
76+
77+
#[test]
78+
fn test_slice_empty_zero_range() {
79+
let env = Env::default();
80+
let v: Vec<u32> = vec![&env];
81+
let sliced = v.slice(0..0);
82+
assert_eq!(sliced.len(), 0);
83+
}

soroban-sdk/src/unwrap.rs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ use core::convert::Infallible;
33
pub trait UnwrapOptimized {
44
type Output;
55
fn unwrap_optimized(self) -> Self::Output;
6+
fn expect_optimized(self, msg: &'static str) -> Self::Output;
67
}
78

89
impl<T> UnwrapOptimized for Option<T> {
@@ -18,6 +19,17 @@ impl<T> UnwrapOptimized for Option<T> {
1819
#[cfg(not(target_family = "wasm"))]
1920
self.unwrap()
2021
}
22+
23+
#[inline(always)]
24+
fn expect_optimized(self, _msg: &'static str) -> Self::Output {
25+
#[cfg(target_family = "wasm")]
26+
match self {
27+
Some(t) => t,
28+
None => core::arch::wasm32::unreachable(),
29+
}
30+
#[cfg(not(target_family = "wasm"))]
31+
self.expect(_msg)
32+
}
2133
}
2234

2335
impl<T, E: core::fmt::Debug> UnwrapOptimized for Result<T, E> {
@@ -33,6 +45,17 @@ impl<T, E: core::fmt::Debug> UnwrapOptimized for Result<T, E> {
3345
#[cfg(not(target_family = "wasm"))]
3446
self.unwrap()
3547
}
48+
49+
#[inline(always)]
50+
fn expect_optimized(self, _msg: &'static str) -> Self::Output {
51+
#[cfg(target_family = "wasm")]
52+
match self {
53+
Ok(t) => t,
54+
Err(_) => core::arch::wasm32::unreachable(),
55+
}
56+
#[cfg(not(target_family = "wasm"))]
57+
self.expect(_msg)
58+
}
3659
}
3760

3861
pub trait UnwrapInfallible {

soroban-sdk/src/vec.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -783,11 +783,15 @@ impl<T> Vec<T> {
783783
pub fn slice(&self, r: impl RangeBounds<u32>) -> Self {
784784
let start_bound = match r.start_bound() {
785785
Bound::Included(s) => *s,
786-
Bound::Excluded(s) => *s + 1,
786+
Bound::Excluded(s) => s
787+
.checked_add(1)
788+
.expect_optimized("attempt to add with overflow"),
787789
Bound::Unbounded => 0,
788790
};
789791
let end_bound = match r.end_bound() {
790-
Bound::Included(s) => *s + 1,
792+
Bound::Included(s) => s
793+
.checked_add(1)
794+
.expect_optimized("attempt to add with overflow"),
791795
Bound::Excluded(s) => *s,
792796
Bound::Unbounded => self.len(),
793797
};

0 commit comments

Comments
 (0)