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.

Looking in /usr/xenocara/xserver/hw/xfree86/parser/scan.c, in xf86getToken(),
there is an XXX comment in the code mentioning that, (for some unspecified
reason), the configRBuf pointer value is being returned in xf86_lex_val.str
instead of a private copy being made, (as is done for, E.G. non-comment
strings).

The comment seems to imply that xf86addComment would handle this gracefully
and not free() the pointer, but clearly the pointer _is_ free'd and the
reasons behind this design choice rather than the seemingly more obvious way
of doing the same malloc and strcpy that is done for regular strings are not
clear.

Indeed, the following cludge does mitigate the issue and the 0xdf bytes 
disappear:

--- scan.c      Thu Oct  5 19:48:29 2023
+++ scan.c.cludge       Thu Oct  5 20:07:06 2023
@@ -335,7 +335,8 @@
             /* XXX no private copy.
              * Use xf86addComment when setting a comment.
              */
-            xf86_lex_val.str = configRBuf;
+               xf86_lex_val.str =malloc(4096);
+               strcpy(xf86_lex_val.str, configRBuf);
             return COMMENT;
         }

 
But presumably there was a reason that this wasn't done all along, so
hopefully maybe somebody who is more familiar with this code can come up with
the propper fix.

Reply via email to