obs-studio icon indicating copy to clipboard operation
obs-studio copied to clipboard

libobs/util: Fix potential memory error in text parser

Open fzwoch opened this issue 1 month ago • 2 comments

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.

fzwoch avatar May 24 '24 11:05 fzwoch