Skip to content

fix(logs): respect printf length modifiers in params #1752

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

Merged
jpnurmi merged 5 commits into from
May 27, 2026
Merged

fix(logs): respect printf length modifiers in params #1752

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7ae8309
fix: respect printf length modifiers in log params
jpnurmi May 25, 2026
bf0553f
Update CHANGELOG.md
jpnurmi May 25, 2026
3397644
typedef printf_ssize/uptrdiff_t
jpnurmi May 25, 2026
4a42325
make format
jpnurmi May 25, 2026
1e59ad8
-Wswitch-enum
jpnurmi May 25, 2026
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
- Native/macOS: fix thread stack descriptor. ([#1726](https://github.com/getsentry/sentry-native/pull/1726))
- Cap rate-limit retry-after values at 24 hours to prevent a MITM-provided response from disabling event delivery for the process lifetime. ([#1744](https://github.com/getsentry/sentry-native/pull/1744))
- Native: validate ELF header entry sizes. ([#1746](https://github.com/getsentry/sentry-native/pull/1746))
- Structured logs: respect printf argument widths when extracting log parameters to avoid stack-data disclosure and corrupted attributes on 32-bit platforms. ([#1752](https://github.com/getsentry/sentry-native/pull/1752))

## 0.14.2

Expand Down
169 changes: 150 additions & 19 deletions src/sentry_logs.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,46 @@
#include "sentry_os.h"
#include "sentry_scope.h"
#include "sentry_value.h"
#include <limits.h>
#include <stdarg.h>
#include <stddef.h>
#include <stdint.h>
#include <string.h>

static sentry_batcher_ref_t g_batcher = SENTRY_BATCHER_REF_INIT;

typedef enum {
PRINTF_LENGTH_NONE,
PRINTF_LENGTH_CHAR,
PRINTF_LENGTH_SHORT,
PRINTF_LENGTH_LONG,
PRINTF_LENGTH_LONG_LONG,
PRINTF_LENGTH_INTMAX,
PRINTF_LENGTH_SIZE,
PRINTF_LENGTH_PTRDIFF,
PRINTF_LENGTH_LONG_DOUBLE,
} printf_length_t;

#if SIZE_MAX == UINT_MAX
typedef int printf_ssize_t;
#elif SIZE_MAX == ULONG_MAX
typedef long printf_ssize_t;
#elif SIZE_MAX == ULLONG_MAX
typedef long long printf_ssize_t;
#else
typedef ptrdiff_t printf_ssize_t;
#endif

#if PTRDIFF_MAX == INT_MAX
typedef unsigned int printf_uptrdiff_t;
#elif PTRDIFF_MAX == LONG_MAX
typedef unsigned long printf_uptrdiff_t;
#elif PTRDIFF_MAX == LLONG_MAX
typedef unsigned long long printf_uptrdiff_t;
#else
typedef size_t printf_uptrdiff_t;
#endif

static const char *
level_as_string(sentry_level_t level)
{
Expand All @@ -32,18 +67,48 @@ level_as_string(sentry_level_t level)
}
}

// TODO to be portable, pass in the length format specifier
#ifndef SENTRY_UNITTEST
static
#endif
sentry_value_t
construct_param_from_conversion(const char conversion, va_list *args_copy)
construct_param_from_conversion(
const char conversion, printf_length_t length, va_list *args_copy)
{
sentry_value_t param_obj = sentry_value_new_object();
switch (conversion) {
case 'd':
case 'i': {
long long val = va_arg(*args_copy, long long);
int64_t val;
switch (length) {
case PRINTF_LENGTH_NONE:
val = va_arg(*args_copy, int);
break;
case PRINTF_LENGTH_CHAR:
val = (signed char)va_arg(*args_copy, int);
break;
case PRINTF_LENGTH_SHORT:
val = (short)va_arg(*args_copy, int);
break;
case PRINTF_LENGTH_LONG:
val = va_arg(*args_copy, long);
break;
case PRINTF_LENGTH_LONG_LONG:
val = va_arg(*args_copy, long long);
break;
case PRINTF_LENGTH_INTMAX:
val = va_arg(*args_copy, intmax_t);
break;
case PRINTF_LENGTH_SIZE:
val = va_arg(*args_copy, printf_ssize_t);
break;
case PRINTF_LENGTH_PTRDIFF:
val = va_arg(*args_copy, ptrdiff_t);
break;
case PRINTF_LENGTH_LONG_DOUBLE:
default:
val = va_arg(*args_copy, int);
break;
}
sentry_value_set_by_key(
param_obj, "value", sentry_value_new_int64(val));
sentry_value_set_by_key(
Expand All @@ -54,12 +119,42 @@ static
case 'x':
case 'X':
case 'o': {
unsigned long long int val = va_arg(*args_copy, unsigned long long int);
uint64_t val;
switch (length) {
case PRINTF_LENGTH_NONE:
val = va_arg(*args_copy, unsigned int);
break;
case PRINTF_LENGTH_CHAR:
val = (unsigned char)va_arg(*args_copy, int);
break;
case PRINTF_LENGTH_SHORT:
val = (unsigned short)va_arg(*args_copy, int);
break;
case PRINTF_LENGTH_LONG:
val = va_arg(*args_copy, unsigned long);
break;
case PRINTF_LENGTH_LONG_LONG:
val = va_arg(*args_copy, unsigned long long);
break;
case PRINTF_LENGTH_INTMAX:
val = va_arg(*args_copy, uintmax_t);
break;
case PRINTF_LENGTH_SIZE:
val = va_arg(*args_copy, size_t);
break;
case PRINTF_LENGTH_PTRDIFF:
val = va_arg(*args_copy, printf_uptrdiff_t);
break;
case PRINTF_LENGTH_LONG_DOUBLE:
default:
val = va_arg(*args_copy, unsigned int);
break;
}
// TODO update once unsigned 64-bit can be sent as non-string
char buf[26];
char format[8];
snprintf(format, sizeof(format), "%%ll%c", conversion);
snprintf(buf, sizeof(buf), format, val);
snprintf(buf, sizeof(buf), format, (unsigned long long)val);
sentry_value_set_by_key(
param_obj, "value", sentry_value_new_string(buf));
sentry_value_set_by_key(
Expand All @@ -72,7 +167,9 @@ static
case 'E':
case 'g':
case 'G': {
double val = va_arg(*args_copy, double);
double val = length == PRINTF_LENGTH_LONG_DOUBLE
? (double)va_arg(*args_copy, long double)
: va_arg(*args_copy, double);
sentry_value_set_by_key(
param_obj, "value", sentry_value_new_double(val));
sentry_value_set_by_key(
Expand Down Expand Up @@ -136,20 +233,28 @@ skip_flags(const char *fmt_ptr)
}

static const char *
skip_width(const char *fmt_ptr)
skip_width(const char *fmt_ptr, va_list *args_copy)
{
if (*fmt_ptr == '*') {
(void)va_arg(*args_copy, int);
return fmt_ptr + 1;
}
while (*fmt_ptr && (*fmt_ptr >= '0' && *fmt_ptr <= '9')) {
fmt_ptr++;
}
return fmt_ptr;
}

static const char *
skip_precision(const char *fmt_ptr)
skip_precision(const char *fmt_ptr, va_list *args_copy)
{

if (*fmt_ptr == '.') {
fmt_ptr++;
if (*fmt_ptr == '*') {
(void)va_arg(*args_copy, int);
return fmt_ptr + 1;
}
while (*fmt_ptr && (*fmt_ptr >= '0' && *fmt_ptr <= '9')) {
fmt_ptr++;
}
Expand All @@ -158,14 +263,39 @@ skip_precision(const char *fmt_ptr)
}

static const char *
skip_length(const char *fmt_ptr)
parse_length(const char *fmt_ptr, printf_length_t *length)
{
while (*fmt_ptr
&& (*fmt_ptr == 'h' || *fmt_ptr == 'l' || *fmt_ptr == 'L'
|| *fmt_ptr == 'z' || *fmt_ptr == 'j' || *fmt_ptr == 't')) {
fmt_ptr++;
*length = PRINTF_LENGTH_NONE;
switch (*fmt_ptr) {
case 'h':
if (*(fmt_ptr + 1) == 'h') {
*length = PRINTF_LENGTH_CHAR;
return fmt_ptr + 2;
}
*length = PRINTF_LENGTH_SHORT;
return fmt_ptr + 1;
case 'l':
if (*(fmt_ptr + 1) == 'l') {
*length = PRINTF_LENGTH_LONG_LONG;
return fmt_ptr + 2;
}
*length = PRINTF_LENGTH_LONG;
return fmt_ptr + 1;
case 'L':
*length = PRINTF_LENGTH_LONG_DOUBLE;
return fmt_ptr + 1;
case 'j':
*length = PRINTF_LENGTH_INTMAX;
return fmt_ptr + 1;
case 'z':
*length = PRINTF_LENGTH_SIZE;
return fmt_ptr + 1;
case 't':
*length = PRINTF_LENGTH_PTRDIFF;
return fmt_ptr + 1;
default:
return fmt_ptr;
}
return fmt_ptr;
}

// returns how many parameters were added to the attributes object
Expand Down Expand Up @@ -197,18 +327,19 @@ static
}

fmt_ptr = skip_flags(fmt_ptr);
fmt_ptr = skip_width(fmt_ptr);
fmt_ptr = skip_precision(fmt_ptr);
fmt_ptr = skip_length(fmt_ptr);
fmt_ptr = skip_width(fmt_ptr, &args_copy);
fmt_ptr = skip_precision(fmt_ptr, &args_copy);
printf_length_t length;
fmt_ptr = parse_length(fmt_ptr, &length);

// Get the conversion specifier
char conversion = *fmt_ptr;
if (conversion) {
char key[64];
snprintf(key, sizeof(key), "sentry.message.parameter.%d",
param_index);
sentry_value_t param_obj
= construct_param_from_conversion(conversion, &args_copy);
sentry_value_t param_obj = construct_param_from_conversion(
conversion, length, &args_copy);
sentry_value_set_by_key(attributes, key, param_obj);
param_index++;
fmt_ptr++;
Expand Down
61 changes: 48 additions & 13 deletions tests/unit/test_logs.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,8 @@ SENTRY_TEST(formatted_log_messages)
sentry_log_warn("Character: %c, Hex: 0x%x", 'A', 255), 0);
TEST_CHECK_INT_EQUAL(sentry_log_error("Pointer: %p", (void *)0x1234), 0);
TEST_CHECK_INT_EQUAL(sentry_log_error("Big number: %zu", UINT64_MAX), 0);
TEST_CHECK_INT_EQUAL(sentry_log_error("Small number: %d", INT64_MIN), 0);
TEST_CHECK_INT_EQUAL(
sentry_log_error("Small number: %" PRId64, INT64_MIN), 0);

sentry_close();

Expand Down Expand Up @@ -166,21 +167,54 @@ test_param_conversion_helper(const char *format, ...)
sentry_value_decref(attributes);
}

static int
populate_test_params(sentry_value_t attributes, const char *format, ...)
{
va_list args;
va_start(args, format);
int param_count = populate_message_parameters(attributes, format, args);
va_end(args);
return param_count;
}

SENTRY_TEST(logs_param_conversion)
{
// TODO this test shows the current limitation for parsing integers on
// 32-bit systems
int a = 1, b = 2, c = 3;
#if defined(__i386__) || defined(_M_IX86) || defined(__arm__)
// Currently, on 32-bit platforms, we need to cast to a 64-bit integer type
// since the parameter conversion expects long long for %d format specifiers
test_param_conversion_helper(
"%" PRId64 " %" PRId64 " %" PRId64, (int64_t)a, (int64_t)b, (int64_t)c);
#else
// since we read these values as 64-bit, this is still undefined behaviour
// but it works because the variadic arguments are passed in 8-byte slots
test_param_conversion_helper("%d %d %d", a, b, c);
#endif
}

SENTRY_TEST(logs_param_sign)
{
sentry_value_t attributes = sentry_value_new_object();
int value = -1;

int param_count = populate_test_params(attributes, "%d", value);

sentry_value_t param
= sentry_value_get_by_key(attributes, "sentry.message.parameter.0");
sentry_value_t param_value = sentry_value_get_by_key(param, "value");

TEST_CHECK_INT_EQUAL(param_count, 1);
TEST_CHECK_INT_EQUAL(sentry_value_as_int64(param_value), -1);

sentry_value_decref(attributes);
}

SENTRY_TEST(logs_param_width)
{
sentry_value_t attributes = sentry_value_new_object();
const char *format = "%*d";

int param_count = populate_test_params(attributes, format, 6, 42);

sentry_value_t param
= sentry_value_get_by_key(attributes, "sentry.message.parameter.0");
sentry_value_t param_value = sentry_value_get_by_key(param, "value");

TEST_CHECK_INT_EQUAL(param_count, 1);
TEST_CHECK_INT_EQUAL(sentry_value_as_int64(param_value), 42);

sentry_value_decref(attributes);
}

static void
Expand Down Expand Up @@ -283,7 +317,8 @@ SENTRY_TEST(logs_param_types)
const char *e = "test";
void *f = (void *)0x12345abc;
uint64_t g = 0xDEADBEEFDEADBEEF;
test_param_conversion_types("%u %d %f %c %s %p %x", a, b, c, d, e, f, g);
test_param_conversion_types(
"%" PRIu64 " %" PRId64 " %f %c %s %p %" PRIx64, a, b, c, d, e, f, g);
}

SENTRY_TEST(logs_force_flush)
Expand Down
2 changes: 2 additions & 0 deletions tests/unit/tests.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,9 @@ XX(logs_custom_attributes_with_format_strings)
XX(logs_disabled)
XX(logs_force_flush)
XX(logs_param_conversion)
XX(logs_param_sign)
XX(logs_param_types)
XX(logs_param_width)
XX(logs_plain_string)
XX(logs_plain_string_disabled)
XX(logs_reinit)
Expand Down
Loading