libobs/util: Fix potential memory error in text parser
Description
Feels weird to touch code that has been there for 11 years.. but I think there is actually a bug in here.
In case the string buffer that is to be parsed is just a single #, lookup_gettoken() reads across the text buffer's memory. After lexer_getbasetoken() is run lex->offset has been incremented to the next character already.
In case of a comment # token lex->offset gets incremented again and is assigned to ch which is then checked for a string termination or newline.
This essentially results in the first byte after # being skipped. This leads to a buffer violation if the the string to parse is being set to #\0:
Essentially we have
char buffer[2];
buffer[0] = '#';
buffer[1] = '\0';
lex->offset = buffer;
lexer_getbasetoken(); // lex->offset now points to buffer[1] ('\0')
// ch is '#', so do comment handling
lex->offset++; // lex->offset now points to buffer[2] which is beyond the text buffer's memory
ch = *lex->offset; // reads from invalid or garbage data
// check if ch is string end or newline
It seems that we get "lucky" most of the time and do not crash reading buffer[2] but read from an existing memory location that seems to be a \0 most of the time.
Surprisingly there are a couple of such situations present in current shipped locale files:
$ find /usr/share/obs/ -size 1c
/usr/share/obs/obs-plugins/frontend-tools/locale/en-GB.ini
/usr/share/obs/obs-plugins/linux-alsa/locale/fil-PH.ini
/usr/share/obs/obs-plugins/linux-capture/locale/en-GB.ini
/usr/share/obs/obs-plugins/linux-jack/locale/en-GB.ini
/usr/share/obs/obs-plugins/linux-pipewire/locale/en-GB.ini
/usr/share/obs/obs-plugins/linux-pulseaudio/locale/en-GB.ini
/usr/share/obs/obs-plugins/linux-pulseaudio/locale/fil-PH.ini
/usr/share/obs/obs-plugins/obs-libfdk/locale/en-GB.ini
/usr/share/obs/obs-plugins/obs-qsv11/locale/en-GB.ini
/usr/share/obs/obs-plugins/obs-vst/locale/en-GB.ini
/usr/share/obs/obs-plugins/obs-webrtc/locale/da-DK.ini
/usr/share/obs/obs-plugins/obs-webrtc/locale/en-GB.ini
/usr/share/obs/obs-plugins/obs-websocket/locale/en-GB.ini
/usr/share/obs/obs-plugins/rtmp-services/locale/az-AZ.ini
/usr/share/obs/obs-plugins/rtmp-services/locale/en-GB.ini
I think other platforms do have these on their platform specific plugins as well.
It probably applies to others too if the last line contains a # without a newline .
Motivation and Context
Not sure what underlying memory we hit here or if its guaranteed to be correctly initialized.. but better not take any chances..
How Has This Been Tested?
I actually discovered it tinkering around with Xcode's "Guard Malloc" feature where OBS Studio suddenly crashes on startup when enabled.
With the change applied no more crashes happen with enabled "Guard Malloc" feature.
Types of changes
- Bug fix (non-breaking change which fixes an issue)
Checklist:
- [x] My code has been run through clang-format.
- [x] I have read the contributing document.
- [x] My code is not on the master branch.
- [x] The code has been tested.
- [x] All commit messages are properly formatted and commits squashed where appropriate.
- [x] I have included updates to all appropriate documentation.
I think at the moment OBS could just crash at startup. Essentially whenever a locale file is loaded that just contains # (no newline, so really with a file size of 1). Not sure if when a plugin loads it loads and parses all locales or just the selected locale, so the language setting may have an impact.
On macOS I believe I was set on en_GB or en_US I believe and the audio encoder or coreaudio plugin was triggering it (recalling from memory, need to check if desired). It happened with several plugins though.
I'm primarily puzzled by the fact there aren't many crash reports on startup. It is possible that the current design results in a memory layout that accidently serves as a safety net.
I think at the moment OBS could just crash at startup. Essentially whenever a locale file is loaded that just contains
#(no newline, so really with a file size of 1). Not sure if when a plugin loads it loads and parses all locales or just the selected locale, so the language setting may have an impact.On macOS I believe I was set on en_GB or en_US I believe and the audio encoder or coreaudio plugin was triggering it (recalling from memory, need to check if desired). It happened with several plugins though.
I'm primarily puzzled by the fact there aren't many crash reports on startup. It is possible that the current design results in a memory layout that accidently serves as a safety net.
Might have been pure luck that the memory around it (the redzone) was allocated as well:
Shadow bytes around the buggy address:
0x0001268daa80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0001268dab00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0001268dab80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0001268dac00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0001268dac80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0001268dad00: fa fa 00 00 00 00[02]fa fa fa 00 00 00 00 02 fa
0x0001268dad80: fa fa 00 00 00 00 00 07 fa fa 00 00 00 00 00 07
0x0001268dae00: fa fa 00 00 00 00 00 fa fa fa 00 00 00 00 00 00
0x0001268dae80: fa fa fd fd fd fd fd fa fa fa 00 00 00 00 00 06
0x0001268daf00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0001268daf80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable: 00
Partially addressable: 01 02 03 04 05 06 07
Heap left redzone: fa
Freed heap region: fd
Stack left redzone: f1
Stack mid redzone: f2
Stack right redzone: f3
Stack after return: f5
Stack use after scope: f8
Global redzone: f9
Global init order: f6
Poisoned by user: f7
Container overflow: fc
Array cookie: ac
Intra object redzone: bb
ASan internal: fe
Left alloca redzone: ca
Right alloca redzone: cb