xf86pciBus: replace on thread-safe strtok_r in xf86ParsePciBusString()

[GermanAizek]

Related to issue: #1398

• Thread A calls xf86ParsePciBusString("PCI:1@0:2:3 "). Execution of strtok_r (in the patched version) or strtok (in the vulnerable version) is suspended after the first call, when the internal pointer is set to the rest of the string ":2:3".
• Thread B (the attacker's thread) calls another function in the X server that also uses strtok (for example, to parse another configuration string). This call overwrites the internal static state of strtok.
• Thread A resumes and calls strtok(NULL, ":") again. Instead of getting the token "2" from the source string, it will receive data from the string processed by Thread B, or NULL, which will lead to incorrect BusID parsing, potential failure, or other unpredictable behavior.

Applying patch with strtok_r completely eliminates this category of vulnerabilities in multithreaded scenarios.

[algrid]

Is there a particular reason to use last for both the first and the last argument?
I thought the typical usage would be something like

char *last;
p = strtok_r(s, ":", &last);

[GermanAizek]

@metux, in XOrg was created SECURITY.md

https://gitlab.freedesktop.org/xorg/xserver/-/commit/a5047d4a6576630d0e15b8f145a44ff6ebe55369

XLibre as a separate fork, would you like to consider creating an isolated environment from the drivers as well? It is possible that someone will write vendor new drivers for the x server in the future, as well as protection against malicious xf86 drivers like viruses.

I can help you create a more secure runtime environment in X process and send PRs.

[GermanAizek]

@metux, As Alan said in SECURITY.md, the Xorg project fully trusts vendors' code and custom modules and eliminates possible errors when writing them that could exploit xorg server vulnerabilities.

[stefan11111]

@GermanAizek I agree with Alanc here.

We want code from modules to be just as fast as code in the X server.
As Alanc told you in the discussion on the Xorg gitlab, we simply dlopen() the modules and execute functions from it.

You can prevent modules doing stuff like *(char*)0 = 0; from bringing down the system by forking and running the code under a different process (though I don't want to think about the many ways this can break).

However, you can't prevent stuff like system ("rm -rf /*"); without some heavy, browser-like sandboxing.

On a properly configured system, X server modules can't be installed without privileges, so this isn't a security vulnerability.

If you think I'm wrong here, try patching a module to create a file somewhere on your system where the X server has enough privileges to do so (do it in driver PreInit or something).
Try then, just by patching the X server, to load the module and make it unable to create that file.