On Thu, Oct 05, 2023 at 08:11:39PM -0300, Crystal Kolipe wrote:
> This is an interesting one...
>
> There is a use after free bug in the X config parser, which can be trivially
> demonstrated by creating a config file with the first line being a comment and
> the second line containing invalid syntax:
>
> $ echo "#foo\nfoo" > custom_config
> $ X -config custom_config
>
> [...]
>
> (++) Using config file: "custom_config"
> (==) Using system config directory "/usr/X11R6/share/X11/xorg.conf.d"
> Parse error on line 3 of section InputClass in file custom_config
> "ßßßßßßßßon" is not a valid keyword in this section.
> (EE)
> Fatal server error:
> (EE) no screens found(EE)
>
> [...]
>
> So part of the error message is reading 0xdf bytes, which are obviously from a
> buffer which has been free'd.
Hi,
I can confirm that there's an issue here. However in my tests, I don't
see a garbled error message.
If I set MALLOC_OPTIONS=F then a double free is reported, which I
tracked down to the realloc() calls in getNextLine() that will make
the xf68_lex_val.str pointer point to already free()d memory.
So the following patch, which invalidates this pointer before reading
more data from the config file fixes the issue I'm seeing. Can you
confirm that it also fixes it for you ?
Index: hw/xfree86/parser/scan.c
===================================================================
RCS file: /cvs/OpenBSD/xenocara/xserver/hw/xfree86/parser/scan.c,v
retrieving revision 1.12
diff -u -p -u -r1.12 scan.c
--- hw/xfree86/parser/scan.c 27 Jul 2019 07:57:18 -0000 1.12
+++ hw/xfree86/parser/scan.c 8 Oct 2023 17:03:21 -0000
@@ -278,9 +278,11 @@ xf86getToken(const xf86ConfigSymTabRec *
if (!c) {
char *ret;
- if (numFiles > 0)
+ if (numFiles > 0) {
+ /* invalidate lex token */
+ xf86_lex_val.str = NULL;
ret = xf86getNextLine();
- else {
+ } else {
if (builtinConfig[builtinIndex] == NULL)
ret = NULL;
else {
--
Matthieu Herrb
