Skip to content

Fix stack buffer overflow in LocalizeRadix()#26490

Open
gladiator9797 wants to merge 2 commits intoprotocolbuffers:mainfrom
gladiator9797:fix-localizeradix-stack-overflow
Open

Fix stack buffer overflow in LocalizeRadix()#26490
gladiator9797 wants to merge 2 commits intoprotocolbuffers:mainfrom
gladiator9797:fix-localizeradix-stack-overflow

Conversation

@gladiator9797
Copy link

Summary

LocalizeRadix() in upb/lex/strtod.c uses strcpy() to copy the remainder of the input string into an 80-byte stack buffer (localized[80]) without bounds checking. When parsing a floating-point number longer than ~78 characters in a locale where the radix character differs from . (e.g., de_DE, fr_FR), the strcpy at line 43 overflows the buffer.

Fix

  • Replace the fixed-size 80-byte stack buffer with a dynamically sized one (stack-allocated up to 256 bytes, heap-allocated for longer strings)
  • Replace strcpy with memcpy using explicit length
  • Pass input_len to LocalizeRadix() to avoid recomputing string length

Reproducer

# Compile with AddressSanitizer
cc -fsanitize=address -fno-omit-frame-pointer upb/lex/strtod.c test.c -o test

# Run in a non-'.' radix locale
LC_ALL=de_DE.UTF-8 ./test

ASAN output:

==88775==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x00016ce7e4d0
WRITE of size 118 at 0x00016ce7e4d0 thread T0
    #0 strcpy
    #1 LocalizeRadix strtod.c:43
    #2 _upb_NoLocaleStrtod strtod.c:63

Test plan

  • ASAN build passes without crash after fix
  • Normal float parsing still works correctly
  • Existing protobuf test suite passes

LocalizeRadix() uses strcpy() to copy the remainder of the input string
into an 80-byte stack buffer without bounds checking. When parsing a
floating-point number longer than ~78 characters in a locale where the
radix character differs from '.', the strcpy overflows the buffer.

The fix replaces the fixed-size stack buffer with a dynamically sized
one (stack-allocated up to 256 bytes, heap-allocated for longer strings),
and uses memcpy with explicit length instead of strcpy.

Reproducer:
  LC_ALL=de_DE.UTF-8
  Parse float string "1.000...001" with >80 characters
  -> AddressSanitizer: stack-buffer-overflow in LocalizeRadix (strtod.c:43)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🅰️ safe for tests Mark a commit as safe to run presubmits over upb

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants