Skip to content

Commit

Permalink
fix: Tighten Stack Usage
Browse files Browse the repository at this point in the history
According to some docs, JVM/JNI stacks on Android can be as small as
32K, and our own sigaltstack is not much larger with 64K.
Make sure to avoid large stack allocations as much as possible.

We have especially seen the literal content of `/proc/self/maps` as well
as formatted addresses inside corrupted release/environment attributes,
which might point to overflows that write into a previously allocated
release/environment string.
  • Loading branch information
Swatinem committed Jun 22, 2021
1 parent b16af1c commit 26b0c06
Show file tree
Hide file tree
Showing 8 changed files with 53 additions and 19 deletions.
2 changes: 1 addition & 1 deletion src/modulefinder/sentry_modulefinder_apple.c
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ remove_image(const struct mach_header *mh, intptr_t UNUSED(vmaddr_slide))
goto done;
}

char ref_addr[100];
char ref_addr[32];
snprintf(ref_addr, sizeof(ref_addr), "0x%llx", (long long)info.dli_fbase);
sentry_value_t new_modules = sentry_value_new_list();

Expand Down
20 changes: 13 additions & 7 deletions src/modulefinder/sentry_modulefinder_linux.c
Original file line number Diff line number Diff line change
Expand Up @@ -435,25 +435,31 @@ load_modules(sentry_value_t modules)
return;
}

// just read the whole map at once, maybe do it line-by-line as a followup…
char buf[4096];
// Read the whole map at once. Doing it line-by-line would be a good
// followup.
sentry_stringbuilder_t sb;
sentry__stringbuilder_init(&sb);
while (true) {
char *buf = sentry__stringbuilder_reserve(&sb, 4096);
if (!buf) {
sentry__stringbuilder_cleanup(&sb);
close(fd);
return;
}
ssize_t n = read(fd, buf, 4096);
if (n < 0 && (errno == EAGAIN || errno == EINTR)) {
continue;
} else if (n <= 0) {
break;
}
if (sentry__stringbuilder_append_buf(&sb, buf, n)) {
sentry__stringbuilder_cleanup(&sb);
close(fd);
return;
}
sentry__stringbuilder_set_len(&sb, sentry__stringbuilder_len(&sb) + n);
}
close(fd);

if (sentry__stringbuilder_append_char(&sb, "\0")) {
sentry__stringbuilder_cleanup(&sb);
return;
}
char *contents = sentry__stringbuilder_into_string(&sb);
if (!contents) {
return;
Expand Down
2 changes: 1 addition & 1 deletion src/sentry_json.c
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ void
sentry__jsonwriter_write_double(sentry_jsonwriter_t *jw, double val)
{
if (can_write_item(jw)) {
char buf[50];
char buf[24];
// The MAX_SAFE_INTEGER is 9007199254740991, which has 16 digits
int written = sentry__snprintf_c(buf, sizeof(buf), "%.16g", val);
// print `null` if we have printf issues or a non-finite double, which
Expand Down
4 changes: 2 additions & 2 deletions src/sentry_os.c
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ sentry__get_os_context(void)
}
ffi->dwFileFlags &= ffi->dwFileFlagsMask;

char buf[100];
char buf[32];
snprintf(buf, sizeof(buf), "%u.%u.%u", ffi->dwFileVersionMS >> 16,
ffi->dwFileVersionMS & 0xffff, ffi->dwFileVersionLS >> 16);

Expand Down Expand Up @@ -71,7 +71,7 @@ sentry__get_os_context(void)

sentry_value_set_by_key(os, "name", sentry_value_new_string("macOS"));

char buf[100];
char buf[32];
size_t buf_len = sizeof(buf);

if (sysctlbyname("kern.osproductversion", buf, &buf_len, NULL, 0) != 0) {
Expand Down
26 changes: 21 additions & 5 deletions src/sentry_string.c
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ sentry__stringbuilder_init(sentry_stringbuilder_t *sb)
sb->len = 0;
}

static int
append(sentry_stringbuilder_t *sb, const char *s, size_t len)
char *
sentry__stringbuilder_reserve(sentry_stringbuilder_t *sb, size_t len)
{
size_t needed = sb->len + len + 1;
size_t needed = sb->len + len;
if (!sb->buf || needed > sb->allocated) {
size_t new_alloc_size = sb->allocated;
if (new_alloc_size == 0) {
Expand All @@ -27,7 +27,7 @@ append(sentry_stringbuilder_t *sb, const char *s, size_t len)
}
char *new_buf = sentry_malloc(new_alloc_size);
if (!new_buf) {
return 1;
return NULL;
}
if (sb->buf) {
memcpy(new_buf, sb->buf, sb->allocated);
Expand All @@ -36,7 +36,17 @@ append(sentry_stringbuilder_t *sb, const char *s, size_t len)
sb->buf = new_buf;
sb->allocated = new_alloc_size;
}
memcpy(sb->buf + sb->len, s, len);
return &sb->buf[sb->len];
}

static int
append(sentry_stringbuilder_t *sb, const char *s, size_t len)
{
char *buf = sentry__stringbuilder_reserve(sb, len + 1);
if (!buf) {
return 1;
}
memcpy(buf, s, len);
sb->len += len;

// make sure we're always zero terminated
Expand Down Expand Up @@ -105,6 +115,12 @@ sentry__stringbuilder_len(const sentry_stringbuilder_t *sb)
return sb->len;
}

void
sentry__stringbuilder_set_len(sentry_stringbuilder_t *sb, size_t len)
{
sb->len = len;
}

char *
sentry__string_clone(const char *str)
{
Expand Down
12 changes: 12 additions & 0 deletions src/sentry_string.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,18 @@ void sentry__stringbuilder_cleanup(sentry_stringbuilder_t *sb);
*/
size_t sentry__stringbuilder_len(const sentry_stringbuilder_t *sb);

/**
* Resizes the stringbuilder buffer to make sure there is at least `len` bytes
* available at the end, and returns a pointer *to the reservation*.
*/
char *sentry__stringbuilder_reserve(sentry_stringbuilder_t *sb, size_t len);

/**
* Sets the number of used bytes in the string builder, to be used together with
* `sentry__stringbuilder_reserve` to avoid copying from an intermediate buffer.
*/
void sentry__stringbuilder_set_len(sentry_stringbuilder_t *sb, size_t len);

/**
* Duplicates a zero terminated string.
*/
Expand Down
2 changes: 1 addition & 1 deletion src/sentry_utils.c
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ sentry__dsn_get_minidump_url(const sentry_dsn_t *dsn)
char *
sentry__msec_time_to_iso8601(uint64_t time)
{
char buf[255];
char buf[64];
size_t buf_len = sizeof(buf);
time_t secs = time / 1000;
struct tm *tm;
Expand Down
4 changes: 2 additions & 2 deletions src/sentry_value.c
Original file line number Diff line number Diff line change
Expand Up @@ -544,7 +544,7 @@ sentry__value_stringify(sentry_value_t value)
case SENTRY_VALUE_TYPE_STRING:
return sentry__string_clone(sentry_value_as_string(value));
default: {
char buf[50];
char buf[24];
size_t written = (size_t)sentry__snprintf_c(
buf, sizeof(buf), "%g", sentry_value_as_double(value));
if (written >= sizeof(buf)) {
Expand Down Expand Up @@ -934,7 +934,7 @@ sentry__value_new_string_from_wstr(const wchar_t *s)
sentry_value_t
sentry__value_new_addr(uint64_t addr)
{
char buf[100];
char buf[32];
size_t written = (size_t)snprintf(
buf, sizeof(buf), "0x%llx", (unsigned long long)addr);
if (written >= sizeof(buf)) {
Expand Down

0 comments on commit 26b0c06

Please sign in to comment.