Skip to content

Commit

Permalink
strncpy out of bounds fix (#1785)
Browse files Browse the repository at this point in the history
* Fix a bug in strncpy and add some tests that verify the behavior

* null-terminate destination buffer in emb_strncpy
  • Loading branch information
priettt authored Jan 6, 2025
1 parent 5ff5b3c commit d95f64c
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 11 deletions.
25 changes: 15 additions & 10 deletions embrace-android-sdk/src/main/cpp/utils/string_utils.c
Original file line number Diff line number Diff line change
@@ -1,19 +1,24 @@
#include "string_utils.h"
#include "../schema/stack_frames.h"

void emb_strncpy(char *dst, const char *src, size_t len) {
if (dst == NULL || src == NULL) {
/**
* Copy a string from source to destination. If the source is larger than the destination, it will be truncated.
* The destination buffer will always be null-terminated.
* @param destination The destination buffer.
* @param source The source buffer.
* @param destination_len The length of the destination buffer.
*/
void emb_strncpy(char *destination, const char *source, size_t destination_len) {
if (destination == NULL || source == NULL || destination_len == 0) {
return;
}
int i = 0;
while (i <= len) {
char current = src[i];
dst[i] = current;
if (current == '\0') {
break;
}
i++;

size_t i;
for (i = 0; i < destination_len - 1 && source[i] != '\0'; i++) {
destination[i] = source[i];
}

destination[i] = '\0'; // Null-terminate the destination buffer.
}

void emb_convert_to_hex_addr(uint64_t addr, char *buffer) {
Expand Down
2 changes: 1 addition & 1 deletion embrace-android-sdk/src/main/cpp/utils/string_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
extern "C" {
#endif

void emb_strncpy(char *dst, const char *src, size_t len);
void emb_strncpy(char *destination, const char *source, size_t destination_len);
void emb_convert_to_hex_addr(uint64_t addr, char *buffer);

#ifdef __cplusplus
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,27 @@ TEST convert_to_hex(void) {
PASS();
}

TEST test_string_copy_larger_source(void) {
char dst[2];
char src[] = "123456";
emb_strncpy(dst, src, sizeof(dst));
ASSERT_STR_EQ("1", dst); // the rest of the source is truncated to size 2 (1 and a null terminator)
PASS();
}

TEST test_string_copy_larger_destination(void) {
char dst[6] = {0};
char src[] = "12";
emb_strncpy(dst, src, sizeof(dst));
ASSERT_STR_EQ("12", dst);
PASS();
}

SUITE(suite_utilities) {
RUN_TEST(string_copy_success);
RUN_TEST(string_null_src);
RUN_TEST(string_null_dst);
RUN_TEST(convert_to_hex);
RUN_TEST(test_string_copy_larger_source);
RUN_TEST(test_string_copy_larger_destination);
}

0 comments on commit d95f64c

Please sign in to comment.