Skip to content

Commit 26eeb44

Browse files
authored
Add bound check functions for strncpy and memcpy (#1117)
1 parent a4f9b00 commit 26eeb44

16 files changed

+252
-2
lines changed

CMakeLists.txt

+5
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,11 @@ if(GCC5_CXX11_ABI_WORKAROUND)
189189
add_definitions(-D_GLIBCXX_USE_CXX11_ABI=0)
190190
endif()
191191

192+
option(USE_ONEDS_BOUNDCHECK_METHODS "Use bound check methods for C99 functions" OFF)
193+
if (USE_ONEDS_BOUNDCHECK_METHODS)
194+
add_definitions(-DHAVE_ONEDS_BOUNDCHECK_METHODS)
195+
endif()
196+
192197
if(PAL_IMPLEMENTATION STREQUAL "WIN32")
193198
add_definitions(-DZLIB_WINAPI)
194199
endif()

lib/bond/CompactBinaryProtocolReader.hpp

+6
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,9 @@
1010

1111
#include <string.h>
1212

13+
#ifdef HAVE_ONEDS_BOUNDCHECK_METHODS
1314
#include "utils/annex_k.hpp"
15+
#endif
1416

1517
namespace bond_lite {
1618

@@ -66,7 +68,11 @@ class CompactBinaryProtocolReader {
6668
if ((data == nullptr) || (size == 0)) {
6769
return false;
6870
}
71+
#ifdef HAVE_ONEDS_BOUNDCHECK_METHODS
72+
bool result = MAT::BoundCheckFunctions::oneds_memcpy_s(static_cast<uint8_t*>(data), size, &(m_input[m_ofs]), size);
73+
#else
6974
bool result = (memcpy_s(static_cast<uint8_t*>(data), size, &(m_input[m_ofs]), size) == 0);
75+
#endif
7076
m_ofs += size;
7177
return result;
7278
}

lib/http/HttpClient_Curl.hpp

+8-1
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,10 @@
3030
#include "IHttpClient.hpp"
3131
#include "pal/PAL.hpp"
3232

33+
#ifdef HAVE_ONEDS_BOUNDCHECK_METHODS
34+
#include "utils/annex_k.hpp"
35+
#endif
36+
3337
#define HTTP_CONN_TIMEOUT 5L
3438
#define HTTP_STATUS_REGEXP "HTTP\\/\\d\\.\\d (\\d+)\\ .*"
3539
#define HTTP_HEADER_REGEXP "(.*)\\: (.*)\\n*"
@@ -490,8 +494,11 @@ class CurlHttpOperation {
490494
TRACE("not enough memory (realloc returned NULL)\n");
491495
return 0;
492496
}
493-
497+
#ifdef HAVE_ONEDS_BOUNDCHECK_METHODS
498+
BoundCheckFunctions::oneds_memcpy_s(&(mem->memory[mem->size]), realsize, contents, realsize);
499+
#else
494500
memcpy(&(mem->memory[mem->size]), contents, realsize);
501+
#endif
495502
mem->size += realsize;
496503
mem->memory[mem->size] = 0;
497504

lib/include/mat/config-compact-dll.h

+1
Original file line numberDiff line numberDiff line change
@@ -22,4 +22,5 @@
2222
#define HAVE_CS3
2323
//#define HAVE_CS4
2424
//#define HAVE_CS4_FULL
25+
//#define HAVE_ONEDS_BOUNDCHECK_METHODS
2526

lib/include/mat/config-compact-exp.h

+1
Original file line numberDiff line numberDiff line change
@@ -22,4 +22,5 @@
2222
#define HAVE_CS3
2323
//#define HAVE_CS4
2424
//#define HAVE_CS4_FULL
25+
//#define HAVE_ONEDS_BOUNDCHECK_METHODS
2526

lib/include/mat/config-compact-min.h

+1
Original file line numberDiff line numberDiff line change
@@ -22,4 +22,5 @@
2222
#define HAVE_CS3
2323
//#define HAVE_CS4
2424
//#define HAVE_CS4_FULL
25+
//#define HAVE_ONEDS_BOUNDCHECK_METHODS
2526

lib/include/mat/config-compact-noutc.h

+1
Original file line numberDiff line numberDiff line change
@@ -22,4 +22,5 @@
2222
#define HAVE_CS3
2323
//#define HAVE_CS4
2424
//#define HAVE_CS4_FULL
25+
//#define HAVE_ONEDS_BOUNDCHECK_METHODS
2526

lib/include/mat/config-compact.h

+1
Original file line numberDiff line numberDiff line change
@@ -22,4 +22,5 @@
2222
#define HAVE_CS3
2323
//#define HAVE_CS4
2424
//#define HAVE_CS4_FULL
25+
//#define HAVE_ONEDS_BOUNDCHECK_METHODS
2526

lib/include/mat/config-default-cs4.h

+1
Original file line numberDiff line numberDiff line change
@@ -36,4 +36,5 @@
3636
//#define HAVE_CS3
3737
#define HAVE_CS4
3838
#define HAVE_CS4_FULL
39+
//#define HAVE_ONEDS_BOUNDCHECK_METHODS
3940

lib/include/mat/config-default-exp.h

+1
Original file line numberDiff line numberDiff line change
@@ -34,4 +34,5 @@
3434
#define HAVE_CS3
3535
//#define HAVE_CS4
3636
//#define HAVE_CS4_FULL
37+
//#define HAVE_ONEDS_BOUNDCHECK_METHODS
3738

lib/include/mat/config-default.h

+1
Original file line numberDiff line numberDiff line change
@@ -37,4 +37,5 @@
3737
#define HAVE_CS3
3838
//#define HAVE_CS4
3939
//#define HAVE_CS4_FULL
40+
//#define HAVE_ONEDS_BOUNDCHECK_METHODS
4041

lib/tracing/api/sha1.c

+17-1
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,10 @@ A million repetitions of "a"
2525

2626
#include "sha1.h"
2727

28+
#ifdef HAVE_ONEDS_BOUNDCHECK_METHODS
29+
#include "utils/annex_k.hpp"
30+
#endif
31+
2832

2933
#define rol(value, bits) (((value) << (bits)) | ((value) >> (32 - (bits))))
3034

@@ -67,8 +71,11 @@ void SHA1Transform(
6771
#ifdef SHA1HANDSOFF
6872
CHAR64LONG16 block[1]; /* use array to appear as a pointer */
6973

70-
memcpy(block, buffer, 64);
74+
#ifdef USE_ONEDS_BOUNDCHECK_METHODS
75+
BoundCheckFunctions::oneds_memcpy_s(block, 64, buffer, 64)
7176
#else
77+
memcpy(block, buffer, 64);
78+
#endif
7279
/* The following had better never be used because it causes the
7380
* pointer-to-const buffer to be cast into a pointer to non-const.
7481
* And the result is written through. I threw a "const" in, hoping
@@ -212,7 +219,12 @@ void SHA1Update(
212219
j = (j >> 3) & 63;
213220
if ((j + len) > 63)
214221
{
222+
#ifdef HAVE_ONEDS_BOUNDCHECK_METHODS
223+
i = 64 - j;
224+
BoundCheckFunctions::oneds_memcpy_s(&context->buffer[j], i, data, i);
225+
#else
215226
memcpy(&context->buffer[j], data, (i = 64 - j));
227+
#endif
216228
SHA1Transform(context->state, context->buffer);
217229
for (; i + 63 < len; i += 64)
218230
{
@@ -222,7 +234,11 @@ void SHA1Update(
222234
}
223235
else
224236
i = 0;
237+
#ifdef USE_ONEDS_BOUNDCHECK_METHODS
238+
BoundCheckFunctions::oneds_memcpy_s(&context->buffer[j], len - i, &data[i], len - i);
239+
#else
225240
memcpy(&context->buffer[j], &data[i], len - i);
241+
#endif
226242
}
227243

228244

lib/utils/annex_k.hpp

+168
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,17 @@
22
// Copyright (c) Microsoft Corporation. All rights reserved.
33
// SPDX-License-Identifier: Apache-2.0
44
//
5+
#include "ctmacros.hpp"
6+
7+
#include <stddef.h>
8+
#include <errno.h>
9+
#include <string.h>
10+
#ifndef _MSC_VER
11+
#include <stdint.h>
12+
#else
13+
#include <limits.h>
14+
#endif
15+
516
#ifndef ANNEX_K_HPP
617
#define ANNEX_K_HPP
718

@@ -18,5 +29,162 @@
1829

1930
#endif
2031

32+
// restrict keyword needs c99 and above.
33+
#if __STDC_VERSION_ < 199901L
34+
# define restrict
35+
#endif
36+
37+
#ifndef RSIZE_MAX
38+
#define RSIZE_MAX (SIZE_MAX >> 1)
39+
typedef size_t rsize_t;
40+
typedef int errno_t;
41+
#endif
42+
43+
namespace MAT_NS_BEGIN
44+
{
45+
class BoundCheckFunctions
46+
{
47+
private:
48+
static bool oneds_buffer_region_overlap(const char *buffer1, size_t buffer1_len, const char *buffer2, size_t buffer2_len)
49+
{
50+
if (buffer2 >= buffer1)
51+
{
52+
if (buffer1 + buffer1_len - 1 > buffer2 )
53+
{
54+
return true;
55+
}
56+
}
57+
else
58+
{
59+
if (buffer2 + buffer2_len - 1 > buffer1)
60+
{
61+
return true;
62+
}
63+
}
64+
return false;
65+
}
66+
67+
public:
68+
// prototype - https://en.cppreference.com/w/c/string/byte/strlen
69+
// Returns the length of the given null-terminated byte string
70+
// - returns zero if str is a null pointer
71+
// - returns strsz if the null character was not found in the first strsz bytes of str.
72+
73+
static size_t oneds_strnlen_s(const char *str, size_t strsz)
74+
{
75+
if ( str == NULL)
76+
{
77+
return 0;
78+
}
79+
return strnlen(str, strsz);
80+
}
81+
82+
// prototype - https://en.cppreference.com/w/c/string/byte/strncpy (strcpy, strcpy_s)
83+
// Copies at most count characters of the character array pointed to by src
84+
//(including the terminating null character, but not any of the characters that follow
85+
// the null character) to character array pointed to by dest.
86+
// Handles error conditions at runtime -
87+
// - src or dest is a null pointer
88+
// - destsz is zero or greater than RSIZE_MAX
89+
// - count is greater than RSIZE_MAX
90+
// - count is greater or equal destsz, but destsz is less or equal strnlen_s(src, count), in other words, truncation would occur
91+
// - overlap would occur between the source and the destination strings
92+
static errno_t oneds_strncpy_s(char * restrict dest, rsize_t destsz, const char *restrict src, rsize_t count)
93+
{
94+
#if (defined __STDC_LIB_EXT1__) || ( defined _MSC_VER)
95+
return strncpy_s(dest, destsz, src, count);
96+
#else
97+
if ((dest == NULL) || (destsz == 0) || (destsz > RSIZE_MAX))
98+
{
99+
return EINVAL;
100+
}
101+
if (src == NULL)
102+
{
103+
dest = NULL;
104+
return EINVAL;
105+
}
106+
if (count > RSIZE_MAX){
107+
dest = NULL;
108+
return EINVAL;
109+
}
110+
if (count >= destsz && destsz <= oneds_strnlen_s(src, count))
111+
{
112+
dest = NULL;
113+
return EINVAL;
114+
}
115+
116+
rsize_t src_len_to_read = oneds_strnlen_s(src, count) + 1 ;
117+
if (src_len_to_read < count)
118+
{
119+
src_len_to_read = count;
120+
}
121+
122+
// donot allow overflow
123+
if (oneds_buffer_region_overlap(dest, destsz, src, src_len_to_read)) {
124+
dest = NULL;
125+
return EINVAL;
126+
}
127+
if (src_len_to_read < destsz)
128+
{
129+
src_len_to_read = destsz;
130+
}
131+
strncpy(dest, src, src_len_to_read);
132+
return 0;
21133
#endif
134+
}
135+
136+
// prototype - https://en.cppreference.com/w/c/string/byte/memcpy
137+
// Copies count characters from the object pointed to by src to the
138+
// object pointed to by dest. Both objects are interpreted as arrays
139+
// of unsigned char.
140+
//
141+
// Handles error conditions at runtime -
142+
// - dest or src is a null pointer
143+
// - destsz or count is greater than RSIZE_MAX
144+
// - count is greater than destsz (buffer overflow would occur)
145+
// - the source and the destination objects overlap
146+
//
147+
// In case of error, the entire destination range [dest, dest+destsz) is zeroed out
148+
// (if both dest and destsz are valid))
22149

150+
static errno_t oneds_memcpy_s( void *restrict dest, rsize_t destsz,
151+
const void *restrict src, rsize_t count )
152+
{
153+
#if (defined __STDC_LIB_EXT1__) || ( defined _MSC_VER)
154+
return memcpy_s(dest, destsz, src, count);
155+
#else
156+
if (dest == NULL)
157+
{
158+
return EINVAL;
159+
}
160+
if (destsz > RSIZE_MAX)
161+
{
162+
return EINVAL;
163+
}
164+
if (src == NULL)
165+
{
166+
memset(dest, 0, destsz);
167+
return EINVAL;
168+
}
169+
if (count > RSIZE_MAX || count > destsz)
170+
{
171+
memset(dest, 0, destsz);
172+
return EINVAL;
173+
}
174+
// donot allow overflow
175+
if (oneds_buffer_region_overlap((char *)dest, destsz, (char *)src, count)) {
176+
memset(dest, 0, destsz);
177+
return EINVAL;
178+
}
179+
void *result = memcpy(dest, src, count);
180+
if (result == (void *)NULL)
181+
{
182+
return -1;
183+
}
184+
return 0;
185+
#endif
186+
}
187+
};
188+
}
189+
MAT_NS_END
190+
#endif

tests/common/SocketTools.hpp

+7
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@
1414
#include <cstring>
1515
#include <cstddef>
1616
#include <algorithm>
17+
#ifdef HAVE_ONEDS_BOUNDCHECK_METHODS
18+
#include "utils/annex_k.hpp"
19+
#endif
1720

1821
#if defined(HAVE_CONSOLE_LOG) && !defined(LOG_DEBUG)
1922
/* Log to console if there's no standard log facility defined */
@@ -139,7 +142,11 @@ class SocketAddr
139142
{
140143
inet4.sin_port = htons(atoi(colon + 1));
141144
char buf[16];
145+
#ifdef HAVE_ONEDS_BOUNDCHECK_METHODS
146+
MAT::BoundCheckFunctions::oneds_memcpy_s(buf, sizeof(buf), addr, std::min<ptrdiff_t>(15, colon - addr));
147+
#else
142148
memcpy(buf, addr, std::min<ptrdiff_t>(15, colon - addr));
149+
#endif
143150
buf[15] = '\0';
144151
::inet_pton(AF_INET, buf, &inet4.sin_addr);
145152
} else

tests/unittests/AnnexKTests.cpp

+32
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
#include "utils/annex_k.hpp"
2+
#include "common/Common.hpp"
3+
4+
using namespace testing;
5+
using namespace MAT;
6+
7+
TEST(AnnexKTests, memcpy_s)
8+
{
9+
volatile size_t dest_size =10;
10+
volatile size_t src_size = 5;
11+
void *dest = malloc(sizeof(char) * dest_size);
12+
void *src = malloc(sizeof(char) * src_size);
13+
EXPECT_EQ(BoundCheckFunctions::oneds_memcpy_s(src, 5, "TEST", 5), 0);
14+
rsize_t dest_len = dest_size;
15+
rsize_t src_len = src_size-1;
16+
17+
// success tests
18+
EXPECT_EQ(BoundCheckFunctions::oneds_memcpy_s(dest, dest_len, src, 0), 0);
19+
EXPECT_EQ(BoundCheckFunctions::oneds_memcpy_s( dest, dest_len, src, src_len + 1), 0);
20+
EXPECT_EQ(strlen((char *)dest), strlen("TEST"));
21+
EXPECT_EQ(BoundCheckFunctions::oneds_memcpy_s(dest, dest_len + 2, src, src_len + 1), 0);
22+
EXPECT_EQ(strlen((char *)dest), strlen("TEST"));
23+
24+
// error tests
25+
EXPECT_EQ(BoundCheckFunctions::oneds_memcpy_s(dest, 3, src, src_len), EINVAL);
26+
EXPECT_EQ(((char *)dest)[0], '\0');
27+
EXPECT_EQ(BoundCheckFunctions::oneds_memcpy_s(NULL, 3, src, src_len), EINVAL);
28+
EXPECT_EQ(BoundCheckFunctions::oneds_memcpy_s( dest, dest_len, NULL, src_len), EINVAL);
29+
EXPECT_EQ(((char *)dest)[0], '\0');
30+
EXPECT_EQ(BoundCheckFunctions::oneds_memcpy_s( dest, dest_len, src, dest_len + 1 ), EINVAL);
31+
EXPECT_EQ(BoundCheckFunctions::oneds_memcpy_s( dest, dest_len, (void *)((char *)dest + 1), src_len + 1 ), EINVAL);
32+
}

tests/unittests/CMakeLists.txt

+1
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ message("--- unittests")
33
set(SRCS
44
AIJsonSerializerTests.cpp
55
AITelemetrySystemTests.cpp
6+
AnnexKTests.cpp
67
BackoffTests_ExponentialWithJitter.cpp
78
BondSplicerTests.cpp
89
ClockSkewManagerTests.cpp

0 commit comments

Comments
 (0)