|
| 1 | +# Security Issue: XER Primitive Decoder Buffer Overflow |
| 2 | + |
| 3 | +## Summary |
| 4 | + |
| 5 | +`Xer_DecodePrimitiveElement` copies XML element content into a caller-supplied buffer without bounds checking. Malformed XER input with oversized element content can overflow fixed-size stack buffers. |
| 6 | + |
| 7 | +## Location |
| 8 | + |
| 9 | +- **File:** `asn1crt/asn1crt_encoding_xer.c` |
| 10 | +- **Function:** `Xer_DecodePrimitiveElement` (lines 248-330) |
| 11 | +- **Exposed via:** `Xer_DecodeString`, `Xer_DecodeOctetString`, `Xer_DecodeObjectIdentifier`, etc. |
| 12 | + |
| 13 | +## Affected Code |
| 14 | + |
| 15 | +```c |
| 16 | +while (c != '<') |
| 17 | +{ |
| 18 | + if (!GetNextChar(pByteStrm, &c)) |
| 19 | + return FALSE; |
| 20 | + if (c == '<') { |
| 21 | + *pDecodedValue = 0x0; |
| 22 | + pDecodedValue++; |
| 23 | + break; |
| 24 | + } |
| 25 | + |
| 26 | + *pDecodedValue = c; // No bounds check |
| 27 | + pDecodedValue++; |
| 28 | +} |
| 29 | +``` |
| 30 | + |
| 31 | +## Impact |
| 32 | + |
| 33 | +- Callers use fixed buffers: 256 bytes (integers), 1024 bytes (octet strings, OIDs), 2048 bytes (bit strings) |
| 34 | +- `Xer_DecodeString` passes through user buffer with unknown size |
| 35 | +- Overflow corrupts stack, may enable code execution or crash |
| 36 | + |
| 37 | +## Prerequisites |
| 38 | + |
| 39 | +1. Application uses `-XER` flag during code generation |
| 40 | +2. Application decodes XER/XML data from untrusted source |
| 41 | +3. Attacker provides element content exceeding buffer size |
| 42 | + |
| 43 | +## CVSS v3.1 Estimate |
| 44 | + |
| 45 | +**Vector:** `AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H` |
| 46 | +**Score:** 7.5 (High) - assuming XER decoder processes network input |
| 47 | + |
| 48 | +If code execution is achievable: `AV:N/AC:H/PR:N/UI:N/S:U/C:H/I:H/A:H` = 8.1 |
| 49 | + |
| 50 | +*Note: Real-world impact depends on whether XER is used with untrusted input.* |
| 51 | + |
| 52 | +## Suggested Fix |
| 53 | + |
| 54 | +```diff |
| 55 | +--- a/asn1crt/asn1crt_encoding_xer.c |
| 56 | ++++ b/asn1crt/asn1crt_encoding_xer.c |
| 57 | +@@ -245,8 +245,9 @@ flag Xer_EncodePrimitiveElement(ByteStream* pByteStrm, const char* elementTag, c |
| 58 | + } |
| 59 | + |
| 60 | + |
| 61 | +-flag Xer_DecodePrimitiveElement(ByteStream* pByteStrm, const char* elementTag, char* pDecodedValue, int *pErrCode) |
| 62 | ++flag Xer_DecodePrimitiveElement(ByteStream* pByteStrm, const char* elementTag, char* pDecodedValue, size_t maxLen, int *pErrCode) |
| 63 | + { |
| 64 | ++ size_t written = 0; |
| 65 | + Token t; |
| 66 | + char c = 0x0; |
| 67 | + |
| 68 | +@@ -288,12 +289,17 @@ flag Xer_DecodePrimitiveElement(ByteStream* pByteStrm, const char* elementTag, c |
| 69 | + while (c != '<') |
| 70 | + { |
| 71 | + if (!GetNextChar(pByteStrm, &c)) |
| 72 | + return FALSE; |
| 73 | + if (c == '<') { |
| 74 | + *pDecodedValue = 0x0; |
| 75 | + break; |
| 76 | + } |
| 77 | ++ if (written >= maxLen - 1) { |
| 78 | ++ *pErrCode = ERR_INVALID_XML_FILE; |
| 79 | ++ return FALSE; |
| 80 | ++ } |
| 81 | + *pDecodedValue = c; |
| 82 | + pDecodedValue++; |
| 83 | ++ written++; |
| 84 | + } |
| 85 | + |
| 86 | + PushBackChar(pByteStrm); |
| 87 | +``` |
| 88 | + |
| 89 | +Update all callers to pass buffer size: |
| 90 | + |
| 91 | +```diff |
| 92 | +--- a/asn1crt/asn1crt_encoding_xer.c |
| 93 | ++++ b/asn1crt/asn1crt_encoding_xer.c |
| 94 | +@@ -707,7 +707,7 @@ flag Xer_DecodeInteger(ByteStream* pByteStrm, const char* elementTag, asn1SccSin |
| 95 | + { |
| 96 | + char tmp[256]; |
| 97 | + memset(tmp, 0x0, sizeof(tmp)); |
| 98 | +- if (!Xer_DecodePrimitiveElement(pByteStrm, elementTag, tmp, pErrCode)) |
| 99 | ++ if (!Xer_DecodePrimitiveElement(pByteStrm, elementTag, tmp, sizeof(tmp), pErrCode)) |
| 100 | + return FALSE; |
| 101 | + *value = atoll(tmp); |
| 102 | + return TRUE; |
| 103 | +``` |
| 104 | + |
| 105 | +## Testing |
| 106 | + |
| 107 | +1. Create XER input with element content > 2048 bytes |
| 108 | +2. Decode using generated XER decoder |
| 109 | +3. Verify decoder returns error instead of crashing |
| 110 | + |
0 commit comments