Uninitialized read/buffer overrun in SmartHighlighter! #1144
Description
This bug was introduced in 037b41e, which merged PR #187. I've just hit this, and so has @chcg. He's even already opened PR #1111 to "fix" it. I say "fix", because the code is pretty silly (with and without his PR), because we're calling strlen on a string of known length!
Here's my detailed analysis, of the specific situation where I encountered this:
First, open "C:\Users\Alexander Riccio\snprintf_recovered_from_gcc_explorer".
Next, open "C:\Users\Alexander Riccio\Documents\GitHub\UIforETW\RetrieveSymbols\RetrieveSymbols.cpp"
Finally, highlight SymFindFileInPath, with ctrl + right.
listChar, before the scintilla API call, is:
+ listChar 0x0c70af38 "ÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍýýýýÐÐÐÐ... char *
pHighlightView->execute(SCI_GETWORDCHARS, 0, (LPARAM)listChar) returns 191
listChar, after the scintilla API call is:
+ listChar 0x0c70af38 "ÿþýüûúùø÷öõôóòñðïîíìëêéèçæåäãâáàßÞÝÜÛÚÙØ×ÖÕÔÓÒÑÐÏÎÍÌËÊÉÈÇÆÅÄÃÂÁÀ¿¾½¼»º¹¸·¶µ´³²±°¯®¬«ª©¨§¦¥¤£¢¡ Ÿž�œ›š™˜—–•”“’‘��Ž�Œ‹Š‰ˆ‡†…„ƒ‚�€zyxwvutsrqponmlkjihgfedcba_ZYXWVUTSRQPONMLKJIHGFEDCBA9876543210ÍýýýýÐÐÐÐ... char *
Before isQualifiedWord(text2Find, listChar) is called, text2Find is:
+ text2Find 0x2b86afe8 "SymFindFileInPath" char *
...and listChar remains:
+ listChar 0x0c70af38 "ÿþýüûúùø÷öõôóòñðïîíìëêéèçæåäãâáàßÞÝÜÛÚÙØ×ÖÕÔÓÒÑÐÏÎÍÌËÊÉÈÇÆÅÄÃÂÁÀ¿¾½¼»º¹¸·¶µ´³²±°¯®¬«ª©¨§¦¥¤£¢¡ Ÿž�œ›š™˜—–•”“’‘��Ž�Œ‹Š‰ˆ‡†…„ƒ‚�€zyxwvutsrqponmlkjihgfedcba_ZYXWVUTSRQPONMLKJIHGFEDCBA9876543210ÍýýýýÐÐÐÐ... char *
Inside SmartHighlighter::isQualifiedWord(const char *str, char *listChar), len is 17:
len 17 unsigned int
...and SmartHighlighter::isWordChar(char ch, char listChar[]) is called as:
isWordChar(str[i], listChar)
listChar is:
+ listChar 0x0c70af38 "ÿþýüûúùø÷öõôóòñðïîíìëêéèçæåäãâáàßÞÝÜÛÚÙØ×ÖÕÔÓÒÑÐÏÎÍÌËÊÉÈÇÆÅÄÃÂÁÀ¿¾½¼»º¹¸·¶µ´³²±°¯®¬«ª©¨§¦¥¤£¢¡ Ÿž�œ›š™˜—–•”“’‘��Ž�Œ‹Š‰ˆ‡†…„ƒ‚�€zyxwvutsrqponmlkjihgfedcba_ZYXWVUTSRQPONMLKJIHGFEDCBA9876543210ÍýýýýÐÐÐÐ... char *
...and str[i] is:
str[i] 83 'S' const char
More importantly, the in-memory representation of listChar is:
0x0C70AF38 ff fe fd fc fb fa f9 f8 f7 f6 f5 f4 f3 f2 f1 f0 ef ee ed ec eb ea e9 e8 e7 ÿþýüûúùø÷öõôóòñðïîíìëêéèç
0x0C70AF51 e6 e5 e4 e3 e2 e1 e0 df de dd dc db da d9 d8 d7 d6 d5 d4 d3 d2 d1 d0 cf ce æåäãâáàßÞÝÜÛÚÙØ×ÖÕÔÓÒÑÐÏÎ
0x0C70AF6A cd cc cb ca c9 c8 c7 c6 c5 c4 c3 c2 c1 c0 bf be bd bc bb ba b9 b8 b7 b6 b5 ÍÌËÊÉÈÇÆÅÄÃÂÁÀ¿...»º.¸·¶µ
0x0C70AF83 b4 b3 b2 b1 b0 af ae ad ac ab aa a9 a8 a7 a6 a5 a4 a3 a2 a1 a0 9f 9e 9d 9c ´..±°¯®.¬«ª©¨§¦¥¤£¢¡ Ÿž.œ
0x0C70AF9C 9b 9a 99 98 97 96 95 94 93 92 91 90 8f 8e 8d 8c 8b 8a 89 88 87 86 85 84 83 .š™˜—–.”“’‘..Ž.Œ.Š.ˆ....ƒ
0x0C70AFB5 82 81 80 7a 79 78 77 76 75 74 73 72 71 70 6f 6e 6d 6c 6b 6a 69 68 67 66 65 ..€zyxwvutsrqponmlkjihgfe
0x0C70AFCE 64 63 62 61 5f 5a 59 58 57 56 55 54 53 52 51 50 4f 4e 4d 4c 4b 4a 49 48 47 dcba_ZYXWVUTSRQPONMLKJIHG
0x0C70AFE7 46 45 44 43 42 41 39 38 37 36 35 34 33 32 31 30 cd fd fd fd fd d0 d0 d0 d0 FEDCBA9876543210ÍýýýýÐÐÐÐ
0x0C70B000 ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? .........................
The allocation as a whole is:
0x0C70AEED 00 00 00 00 00 00 00 00 00 00 00 bb bb cd ab 00 10 43 05 e4 00 00 00 00 10 ...........»»Í«..C.ä.....
0x0C70AF06 00 00 00 00 00 00 00 00 00 00 8c 43 4e 04 bb bb ba dc c8 af 86 2b 00 00 00 ..........ŒCN.»»ºÜȯ.+...
0x0C70AF1F 00 00 00 00 00 00 00 00 00 c0 00 00 00 01 00 00 00 71 bb 00 00 fd fd fd fd .........À.......q»..ýýýý
0x0C70AF38 ff fe fd fc fb fa f9 f8 f7 f6 f5 f4 f3 f2 f1 f0 ef ee ed ec eb ea e9 e8 e7 ÿþýüûúùø÷öõôóòñðïîíìëêéèç
0x0C70AF51 e6 e5 e4 e3 e2 e1 e0 df de dd dc db da d9 d8 d7 d6 d5 d4 d3 d2 d1 d0 cf ce æåäãâáàßÞÝÜÛÚÙØ×ÖÕÔÓÒÑÐÏÎ
0x0C70AF6A cd cc cb ca c9 c8 c7 c6 c5 c4 c3 c2 c1 c0 bf be bd bc bb ba b9 b8 b7 b6 b5 ÍÌËÊÉÈÇÆÅÄÃÂÁÀ¿...»º.¸·¶µ
0x0C70AF83 b4 b3 b2 b1 b0 af ae ad ac ab aa a9 a8 a7 a6 a5 a4 a3 a2 a1 a0 9f 9e 9d 9c ´..±°¯®.¬«ª©¨§¦¥¤£¢¡ Ÿž.œ
0x0C70AF9C 9b 9a 99 98 97 96 95 94 93 92 91 90 8f 8e 8d 8c 8b 8a 89 88 87 86 85 84 83 .š™˜—–.”“’‘..Ž.Œ.Š.ˆ....ƒ
0x0C70AFB5 82 81 80 7a 79 78 77 76 75 74 73 72 71 70 6f 6e 6d 6c 6b 6a 69 68 67 66 65 ..€zyxwvutsrqponmlkjihgfe
0x0C70AFCE 64 63 62 61 5f 5a 59 58 57 56 55 54 53 52 51 50 4f 4e 4d 4c 4b 4a 49 48 47 dcba_ZYXWVUTSRQPONMLKJIHG
0x0C70AFE7 46 45 44 43 42 41 39 38 37 36 35 34 33 32 31 30 cd fd fd fd fd d0 d0 d0 d0 FEDCBA9876543210ÍýýýýÐÐÐÐ
0x0C70B000 ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? .........................
The bb bb cd ab in the first line is the Page Heap prefix StartStamp, the bb bb ba dc in the second is the prefix EndStamp, and (most importantly) the d0 d0 d0 d0 in the second-to-last-line is the Page Heap end stamp, at the very end of the page.
The ?? bytes at the end are the beginning of the next page (it's inaccessible). Most importantly, there are NO NULL BYTES in the buffer, as it's uninitialized.
Thus, at the line:
for (size_t i = 0, len = strlen(listChar) ; i < len ; ++i)
...strlen overruns the buffer.