Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

strncpy out of bounds fix #1785

Open
wants to merge 2 commits into
base: priettt/ndkCrashFramesFix
Choose a base branch
from
+34 −11
Open

strncpy out of bounds fix #1785

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d18c9a1
Fix a bug in strncpy and add some tests that verify the behavior
priettt Dec 28, 2024
99038bb
null-terminate destination buffer in emb_strncpy
priettt Jan 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 11 additions & 5 deletions embrace-android-sdk/src/main/cpp/utils/string_utils.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,20 @@
#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.
* @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) {
return;
}
int i = 0;
while (i <= len) {
char current = src[i];
dst[i] = current;
while (i < destination_len && source[i] != '\0') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change means the null termination character is not copied to the destination array, which could cause various problems downstream as lots of C code assumes that strings must have that character.

char current = source[i];
destination[i] = current;
if (current == '\0') {
break;
}
Expand Down
2 changes: 1 addition & 1 deletion embrace-android-sdk/src/main/cpp/utils/string_utils.h
View file
Open in desktop
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
18 changes: 18 additions & 0 deletions embrace-android-sdk/src/test/cpp/testcases/utilities/test_string_utils.c
View file
Open in desktop
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("12", dst);
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);
}
Loading