From db150d40fc1d724cf5c0a22e30400245ce86e6dc Mon Sep 17 00:00:00 2001 From: Yao Qi Date: Wed, 24 Jun 2026 10:35:52 +0100 Subject: [PATCH] [Support] Fix undefined shift in decodeULEB128/decodeSLEB128 for overlong encodings When a (signed or unsigned) LEB128 value is encoded with extra trailing bytes that only carry zero- or sign-extension, the decode loop could keep running with the shift amount at 64 or beyond and then evaluate `Slice << Shift`, which is undefined behavior for a 64-bit type. The DWARF expression parser feeds attacker-controlled LEB128 operands (such as `DW_OP_bregN` / `DW_OP_constu`) through `DataExtractor::getULEB128` / `getSLEB128`, so the `lldb-dwarf-expression-fuzzer` reaches this under UBSan. The unsigned case: ``` LEB128.h:152:20: runtime error: shift exponent 70 is too large for 64-bit type 'uint64_t' (aka 'unsigned long long') #0 llvm::decodeULEB128(...) LEB128.h:152 #1 getLEB128(...) DataExtractor.cpp:227 #2 llvm::DataExtractor::getULEB128(...) DataExtractor.cpp:241 #3 llvm::DWARFExpression::Operation::extract(...) DWARFExpression.cpp:218 #7 lldb_private::DWARFExpression::Evaluate(...) DWARFExpression.cpp:1333 #9 LLVMFuzzerTestOneInput lldb-dwarf-expression-fuzzer.cpp:83 SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior LEB128.h:152:20 ``` and the signed case, reached the same way: ``` LEB128.h:190:20: runtime error: shift exponent 70 is too large for 64-bit type 'uint64_t' (aka 'unsigned long long') #0 llvm::decodeSLEB128(...) LEB128.h:190 #1 getLEB128(...) DataExtractor.cpp:227 #2 llvm::DataExtractor::getSLEB128(...) DataExtractor.cpp:245 #3 llvm::DWARFExpression::Operation::extract(...) DWARFExpression.cpp:216 #7 lldb_private::DWARFExpression::Evaluate(...) DWARFExpression.cpp:1333 ``` The existing range checks already guarantee that once `Shift` reaches 64 the remaining bytes are pure extension and contribute nothing to the result, so skip the accumulating shift in that case. Decoded values are unchanged for all well-formed inputs. Adds overlong-encoding regression cases to `LEB128Test`. Without the fix the signed case is the UBSan diagnostic above in a sanitizer build, and in a normal build the unsigned case also decodes to the wrong value (the overlong `1` decodes as `11`). --- llvm/include/llvm/Support/LEB128.h | 14 ++++++++++++-- llvm/unittests/Support/LEB128Test.cpp | 11 +++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/llvm/include/llvm/Support/LEB128.h b/llvm/include/llvm/Support/LEB128.h index 0b8cb6781aec4..bf45131582005 100644 --- a/llvm/include/llvm/Support/LEB128.h +++ b/llvm/include/llvm/Support/LEB128.h @@ -149,7 +149,12 @@ inline uint64_t decodeULEB128(const uint8_t *p, unsigned *n = nullptr, Value = 0; break; } - Value += Slice << Shift; + // Once Shift reaches 64 the remaining bytes have already been validated + // above to be pure zero-extension, so they contribute nothing. Performing + // "Slice << Shift" with Shift >= 64 would be undefined behavior, so skip + // it. + if (LLVM_LIKELY(Shift < 64)) + Value += Slice << Shift; Shift += 7; } while (*p++ >= 128); if (n) @@ -187,7 +192,12 @@ inline int64_t decodeSLEB128(const uint8_t *p, unsigned *n = nullptr, *n = (unsigned)(p - orig_p); return 0; } - Value |= Slice << Shift; + // Once Shift reaches 64 the remaining bytes have already been validated + // above to be pure sign-extension, so they contribute nothing. Performing + // "Slice << Shift" with Shift >= 64 would be undefined behavior, so skip + // it. + if (LLVM_LIKELY(Shift < 64)) + Value |= Slice << Shift; Shift += 7; ++p; } while (Byte >= 128); diff --git a/llvm/unittests/Support/LEB128Test.cpp b/llvm/unittests/Support/LEB128Test.cpp index 0c54a2846903b..dc80f7c4e71ea 100644 --- a/llvm/unittests/Support/LEB128Test.cpp +++ b/llvm/unittests/Support/LEB128Test.cpp @@ -139,6 +139,11 @@ TEST(LEB128Test, DecodeULEB128) { EXPECT_DECODE_ULEB128_EQ(0x80000000'00000000ul, "\x80\x80\x80\x80\x80\x80\x80\x80\x80\x01"); + // Decode overlong ULEB128 whose trailing zero-extension bytes push the shift + // amount to 64 or beyond. + EXPECT_DECODE_ULEB128_EQ(0u, "\x80\x80\x80\x80\x80\x80\x80\x80\x80\x80\x00"); + EXPECT_DECODE_ULEB128_EQ(1u, "\x81\x80\x80\x80\x80\x80\x80\x80\x80\x80\x00"); + #undef EXPECT_DECODE_ULEB128_EQ } @@ -216,6 +221,12 @@ TEST(LEB128Test, DecodeSLEB128) { EXPECT_DECODE_SLEB128_EQ(INT64_MAX, "\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\x00"); + // Decode overlong SLEB128 whose trailing sign-extension bytes push the shift + // amount to 64 or beyond. + EXPECT_DECODE_SLEB128_EQ(0L, "\x80\x80\x80\x80\x80\x80\x80\x80\x80\x80\x00"); + EXPECT_DECODE_SLEB128_EQ(-1L, "\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\x7F"); + EXPECT_DECODE_SLEB128_EQ(-2L, "\xFE\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\x7F"); + #undef EXPECT_DECODE_SLEB128_EQ }