Skip to content

Commit 69306c5

Browse files
committed
Add support for rlbox::strncpy
1 parent 244f869 commit 69306c5

File tree

2 files changed

+136
-9
lines changed

2 files changed

+136
-9
lines changed

code/include/rlbox_stdlib.hpp

Lines changed: 47 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -141,9 +141,9 @@ static constexpr bool can_type_be_memcopied =
141141
std::is_same_v<char16_t, std::remove_cv_t<T>> || std::is_same_v<short, std::remove_cv_t<T>>;
142142

143143
/**
144-
* @brief Copy to sandbox memory area. Note that memcpy is meant to be called on
145-
* byte arrays does not adjust data according to ABI differences. If the
146-
* programmer does accidentally call memcpy on buffers that needs ABI
144+
* @brief Copy buffer to sandbox memory area. Note that memcpy is meant to be
145+
* called on byte arrays does not adjust data according to ABI differences. If
146+
* the programmer does accidentally call memcpy on buffers that needs ABI
147147
* adjustment, this may cause compatibility issues, but will not cause a
148148
* security issue as the destination is always a tainted or tainted_volatile
149149
* pointer
@@ -184,6 +184,50 @@ inline T_Wrap<T_Rhs*, T_Sbx> memcpy(rlbox_sandbox<T_Sbx>& sandbox,
184184
return dest;
185185
}
186186

187+
/**
188+
* @brief Copy string to sandbox memory area. Note that memcpy is meant to be
189+
* called on byte arrays does not adjust data according to ABI differences. If
190+
* the programmer does accidentally call memcpy on buffers that needs ABI
191+
* adjustment, this may cause compatibility issues, but will not cause a
192+
* security issue as the destination is always a tainted or tainted_volatile
193+
* pointer
194+
*/
195+
template<typename T_Sbx,
196+
typename T_Rhs,
197+
typename T_Lhs,
198+
typename T_Num,
199+
template<typename, typename>
200+
typename T_Wrap>
201+
inline T_Wrap<T_Rhs*, T_Sbx> strncpy(rlbox_sandbox<T_Sbx>& sandbox,
202+
T_Wrap<T_Rhs*, T_Sbx> dest,
203+
T_Lhs src,
204+
T_Num num)
205+
{
206+
207+
static_assert(detail::rlbox_is_tainted_or_vol_v<T_Wrap<T_Rhs, T_Sbx>>,
208+
"strncpy called on non wrapped type");
209+
210+
static_assert(!std::is_const_v<T_Rhs>, "Destination is const");
211+
212+
auto num_val = detail::unwrap_value(num);
213+
detail::dynamic_check(num_val <= sandbox.get_total_memory(),
214+
"Called strncpy for memory larger than the sandbox");
215+
216+
tainted<T_Rhs*, T_Sbx> dest_tainted = dest;
217+
char* dest_start = dest_tainted.INTERNAL_unverified_safe();
218+
detail::check_range_doesnt_cross_app_sbx_boundary<T_Sbx>(dest_start, num_val);
219+
220+
// src also needs to be checked, as we don't want to allow a src rand to start
221+
// inside the sandbox and end outside, and vice versa
222+
// src may or may not be a wrapper, so use unwrap_value
223+
const char* src_start = detail::unwrap_value(src);
224+
detail::check_range_doesnt_cross_app_sbx_boundary<T_Sbx>(src_start, num_val);
225+
226+
std::strncpy(dest_start, src_start, num_val);
227+
228+
return dest;
229+
}
230+
187231
/**
188232
* @brief Compare data in sandbox memory area.
189233
*/

code/tests/rlbox/test_stdlib.cpp

Lines changed: 89 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -131,22 +131,29 @@ TEST_CASE("test memcpy", "[stdlib]")
131131

132132
const uint32_t max32Val = 0xFFFFFFFF;
133133

134-
auto dest = sandbox.malloc_in_sandbox<unsigned int>(12); // NOLINT
135-
auto dest_fifth = dest + 4;
134+
/////////////// Check with a tainted source
136135

137-
// tainted src
136+
// Alloc and zero initialize 12 int dest buffer
137+
auto dest = sandbox.malloc_in_sandbox<unsigned int>(12); // NOLINT
138138
for (int i = 0; i < 12; i++) { // NOLINT
139139
*(dest + i) = 0;
140140
}
141+
142+
// Alloc and max initialize 12 int src buffer
141143
auto src = sandbox.malloc_in_sandbox<unsigned int>(12); // NOLINT
142-
auto src_fifth = src + 4;
143144
for (int i = 0; i < 12; i++) { // NOLINT
144145
*(src + i) = max32Val;
145146
}
147+
148+
auto dest_fifth = dest + 4;
149+
auto src_fifth = src + 4;
150+
146151
memcpy(sandbox,
147152
dest_fifth,
148153
src_fifth,
149154
sizeof(tainted<unsigned int, TestSandbox>) * 4);
155+
156+
// Check that destination looks as expected
150157
for (int i = 0; i < 4; i++) { // NOLINT
151158
REQUIRE(*((dest + i).UNSAFE_unverified()) == 0);
152159
}
@@ -157,16 +164,26 @@ TEST_CASE("test memcpy", "[stdlib]")
157164
REQUIRE(*((dest + i).UNSAFE_unverified()) == 0);
158165
}
159166

160-
// untainted src
167+
/////////////// Check with a untainted source
168+
169+
// zero initialize 12 int dest buffer
170+
for (int i = 0; i < 12; i++) { // NOLINT
171+
*(dest + i) = 0;
172+
}
173+
174+
// Alloc and max initialize 12 int untainted src buffer
161175
unsigned int* src2 = new unsigned int[12]; // NOLINT
162-
auto src2_fifth = src2 + 4; // NOLINT
163176
for (int i = 0; i < 12; i++) { // NOLINT
164177
*(src2 + i) = max32Val; // NOLINT
165178
}
179+
180+
auto src2_fifth = src2 + 4; // NOLINT
166181
memcpy(sandbox,
167182
dest_fifth,
168183
src2_fifth,
169184
sizeof(tainted<unsigned int, TestSandbox>) * 4);
185+
186+
// Check that destination looks as expected
170187
for (int i = 0; i < 4; i++) { // NOLINT
171188
REQUIRE(*((dest + i).UNSAFE_unverified()) == 0);
172189
}
@@ -184,6 +201,72 @@ TEST_CASE("test memcpy", "[stdlib]")
184201
sandbox.destroy_sandbox();
185202
}
186203

204+
// NOLINTNEXTLINE
205+
TEST_CASE("test strncpy", "[stdlib]")
206+
{
207+
rlbox::rlbox_sandbox<TestSandbox> sandbox;
208+
sandbox.create_sandbox();
209+
210+
const uint32_t max32Val = 0xFFFFFFFF;
211+
212+
/////////////// Check with a tainted source
213+
214+
// Alloc and zero initialize dest string
215+
auto dest = sandbox.malloc_in_sandbox<char>(12); // NOLINT
216+
for (int i = 0; i < 12; i++) { // NOLINT
217+
*(dest + i) = 0;
218+
}
219+
220+
// Alloc and max initialize src string
221+
auto src = sandbox.malloc_in_sandbox<char>(12); // NOLINT
222+
memcpy(src.unverified_safe_pointer_because(12, "Known size"), "Hello", 6);
223+
224+
strncpy(sandbox,
225+
dest,
226+
src,
227+
12);
228+
229+
// Check that destination looks as expected
230+
REQUIRE(
231+
strncmp(
232+
dest.unverified_safe_pointer_because(12, "Known size"),
233+
src.unverified_safe_pointer_because(12, "Known size"),
234+
6
235+
) == 0
236+
);
237+
238+
/////////////// Check with an untainted source
239+
240+
// zero initialize dest string
241+
for (int i = 0; i < 12; i++) { // NOLINT
242+
*(dest + i) = 0;
243+
}
244+
245+
// Alloc and max initialize src string
246+
auto src2 = new char[12]; // NOLINT
247+
memcpy(src2, "Hello", 6);
248+
249+
strncpy(sandbox,
250+
dest,
251+
src,
252+
12);
253+
254+
// Check that destination looks as expected
255+
REQUIRE(
256+
strncmp(
257+
dest.unverified_safe_pointer_because(12, "Known size"),
258+
src2,
259+
6
260+
) == 0
261+
);
262+
263+
delete[] src2; // NOLINT
264+
sandbox.free_in_sandbox(src);
265+
sandbox.free_in_sandbox(dest);
266+
267+
sandbox.destroy_sandbox();
268+
}
269+
187270
static int normalize(int a)
188271
{
189272
if (a > 0) {

0 commit comments

Comments
 (0)