Skip to content

Commit 3890521

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 146bf29 commit 3890521

File tree

8 files changed

+321
-6
lines changed

8 files changed

+321
-6
lines changed

soroban-sdk/src/bytes.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -713,11 +713,15 @@ impl Bytes {
713713
pub fn slice(&self, r: impl RangeBounds<u32>) -> Self {
714714
let start_bound = match r.start_bound() {
715715
Bound::Included(s) => *s,
716-
Bound::Excluded(s) => *s + 1,
716+
Bound::Excluded(s) => s
717+
.checked_add(1)
718+
.expect_optimized("attempt to add with overflow"),
717719
Bound::Unbounded => 0,
718720
};
719721
let end_bound = match r.end_bound() {
720-
Bound::Included(s) => *s + 1,
722+
Bound::Included(s) => s
723+
.checked_add(1)
724+
.expect_optimized("attempt to add with overflow"),
721725
Bound::Excluded(s) => *s,
722726
Bound::Unbounded => self.len(),
723727
};

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
@@ -4,6 +4,7 @@ mod address;
44
mod auth;
55
mod bytes_alloc_vec;
66
mod bytes_buffer;
7+
mod bytes_slice;
78
mod cmp_across_env_in_tests;
89
mod contract_add_i32;
910
mod contract_assert;
@@ -35,8 +36,10 @@ mod crypto_sha256;
3536
mod env;
3637
mod max_ttl;
3738
mod prng;
39+
mod prng_range;
3840
mod proptest_scval_cmp;
3941
mod proptest_val_cmp;
4042
mod storage_testutils;
4143
mod token_client;
4244
mod token_spec;
45+
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
@@ -750,11 +750,15 @@ impl<T> Vec<T> {
750750
pub fn slice(&self, r: impl RangeBounds<u32>) -> Self {
751751
let start_bound = match r.start_bound() {
752752
Bound::Included(s) => *s,
753-
Bound::Excluded(s) => *s + 1,
753+
Bound::Excluded(s) => s
754+
.checked_add(1)
755+
.expect_optimized("attempt to add with overflow"),
754756
Bound::Unbounded => 0,
755757
};
756758
let end_bound = match r.end_bound() {
757-
Bound::Included(s) => *s + 1,
759+
Bound::Included(s) => s
760+
.checked_add(1)
761+
.expect_optimized("attempt to add with overflow"),
758762
Bound::Excluded(s) => *s,
759763
Bound::Unbounded => self.len(),
760764
};

0 commit comments

Comments
 (0)