Protocol handler

[stefansundin]

This lets the user click a link in a web browser to very easily join a Quake 3 multiplayer game. As browser-based matchmaking websites become more popular, this makes it a lot more convenient and simple to play Quake 3 with others.

The links have the following URI format: quake3://connect/example.com:27950. The format has been designed to be flexible to allow more types of links in the future and avoiding having to make a breaking change. At the moment, connect is the only supported command.

Just imagine if each of the servers on the page https://www.quakeservers.net/quake3/servers/ had a button next to it that said Join Server. I think that would make it easier to both attract new players, and keep them engaged in the longer term.

When you click a link, you will see a prompt like the following:

Just click Open Link and you'll be connected to the server. Simple!

An example of a mod/website that already does this is https://efservers.com/. The only difficulty there is that you have to make further modifications to your system and install a program that sits in between the website and the game. If we can get protocol handler support added directly into ioq3 then the steps that a user has to follow is minimized and the whole process is simpler and more user friendly.

The efservers website has only supported Windows. Recently I contributed handlers for Linux and macOS. My hope is that we can get this added to ioq3, and then I will work to update Lilium Voyager and Tulip Voyager in a way that supports this new protocol handler format, in addition to the current format that efservers is currently using.

I have tested this on Windows, Linux, and macOS, and it works very well on all of them. For Windows you need to create registry entries (the installer creates them if you don't unselect "Protocol Handler"). For Linux you need to install the updated .desktop file. For macOS you just need to drop the bundle in /Applications like normal.

The second commit adds descriptions to the Windows installer. I think I really need that to explain what a "Protocol handler" is, and I like the result.

As a final note, I found it extremely hard to figure out how to compile and build on Windows. Even worse since the wiki is down. I eventually found it on archive.org (https://web.archive.org/web/20190506050817/http://wiki.ioquake3.org/Building_ioQuake3). I first tried to use Cygwin but failed. Then I tried msys2 and got it working but I had to mess with a lot of files. Lots that could be improved here. I should probably have abandoned my attempts and just relied on GitHub Actions for Windows builds. 🤷

Please let me know what you think. :)

[stefansundin]

You can also install the protocol handler on Windows by applying this .reg file. I couldn't upload with a .reg extension so simply rename it from .reg.txt to .reg. You may need to edit it first though as it assumes your ioquake3 executable is located at C:\Games\ioquake3\ioquake3.x86_64.exe.

ioquake3-protocol-handler.reg.txt

[kungfooman]

I first tried to use Cygwin but failed. Then I tried msys2 and got it working but I had to mess with a lot of files. Lots that could be improved here. I should probably have abandoned my attempts and just relied on GitHub Actions for Windows builds. 🤷

Yea, the situation is kinda annoying, not even the Windows build on GitHub works here (missing zip bug). My PR to fix the Visual Studio solution is also not merged e.g.

This project could probably benefit from more maintainers, that want to review additions and then merge.

At least everyone else can review and leave their opinions, too, to help brainstorm the side effects of new PR's plus debugging them.

[ghost]

@stefansundin
Hi, the only way to compile ioq3 for me is using Cygwin. I never got Msys working. Msys is a horrible mess, I don't even know what their actual/latest builds are, and where to get them.
Anyways, what helped me a lot was this nice 'tutorial' how to compile with Cygwin: https://github.com/iortcw/iortcw/blob/master/HOWTO-Build.txt#L19
Following this step-by-step enabled me to compile ioq3, Spearmint, iortcw, Q3e, Tremulous, Open Arena etc.
Try it! Even if it is a bit outdated, it should still work.
Nevertheless, there is no guarantee that it will work :)

[ghost]

@maintainers Why not add such a short HOWTO-Build txt file (or included in the current README) just as a basic tutorial.
Personally I think it would be helpful for many people to have at least a basic help, than waiting for ages for the wiki.
Thanks goes to MAN-AT-ARMS, who provided that tutorial.

[NuclearMonster]

Please keep this PR to discussing the PR, I'm glad for protocol handler support it's something we should have had a long time ago. Wiki movement status is here: https://discourse.ioquake.org/t/moving-the-wiki-content-to-wordpress/1606/

[stefansundin]

Please let me know if there's anything else in this that should be changed.

Is there a process for moving this forward or even merging it? Or do we leave it like this until there's free time for the maintainers to take a deeper look?

Thanks!

[timangus]

Looks good, in general. Style wise it's a bit inconsistent (e.g. brace position); try and match the prevailing style in the file you're changing.

One thing I think you should change is to avoid the __APPLE__ guards on Sys_InitProtocolHandler, instead just provide empty implementations on non-macOS platforms.

I don't know if installing the handler should be optional on Windows? Seems unlikely you would not want to have it, and it isn't really harmful if you don't use it. That's perhaps something of a wider discussion though, I have no strong feelings there.

[stefansundin]

Looks good, in general. Style wise it's a bit inconsistent (e.g. brace position); try and match the prevailing style in the file you're changing.

Ah, good point, I'll try to match it better.

One thing I think you should change is to avoid the __APPLE__ guards on Sys_InitProtocolHandler, instead just provide empty implementations on non-macOS platforms.

Ok, will give this a try too.

I don't know if installing the handler should be optional on Windows? Seems unlikely you would not want to have it, and it isn't really harmful if you don't use it. That's perhaps something of a wider discussion though, I have no strong feelings there.

I had the same thought regarding the various optional DLLs. Why not always install SDL and OpenAL stuff as part of the required ioquake3 section? 🤷

[timangus]

I suspect the SDL option might be a historical thing from the before times when we didn't have SDL working on Windows? I don't actually know though. I think curl and OpenAL are separated out just so they can be conditionally enabled, but that could probably be done in a more fine grained way like the rest of the libraries. But yeah I would just install the protocol handler without asking, other opinions may differ. I haven't touched ioq3 for quite some time now so my opinion is probably worth less that it used to.

[stefansundin]

I guess the only benefit of having it in the installer is that it might bring some awareness to the feature. Otherwise most people won't know that it is there.

Thinking about it more, it is more important to make sure that admins of server directories such as quakeservers.net learn about the feature so that they can add buttons to their websites. But before they can do that there has to be a new release of ioquake3 so that users can actually install a version that supports the protocol handler.

[mgerhardy]

FYI: i've integrated this into worldofpadman: PadWorld-Entertainment/worldofpadman@8efa0b3 Thanks a lot.

[stefansundin]

@mgerhardy Looks great! I'm guessing Join buttons will eventually appear on https://worldofpadman.net/en/servers/? :)

[stefansundin]

@timangus I pushed some code style updates. Let me know if you can think of anything else.

[zturtleman]

IPv6

This is missing support to frag on the net of the future for IPv6 addresses which are enclosed in [] to separate from port like scheme://[2001:db8::7] and scheme://[2001:db8::7]:port. There may already be some Net_* function for this.

-std=gnu89

Newer GCC changed default C standard from gnu89 to gnu99. make CFLAGS="-std=gnu89" should continue to compile without additional warnings and errors. (Whether ioquake3 should target C89 or C99 is a separate issue.)

Variable declarations should be at the beginning of a {} block and not in for (). Function declaration without arguments should have ( void ).

strstr

strstr( var, "needle" ) == var is an interesting solution to check prefix of var but the rest of the code base uses something like !Q_strncmp( var, "needle", the-needle-length ).

[stefansundin]

Thanks @zturtleman, will look into your suggestions. Definitely want to make sure this works on the net of the future. 👍

[ghost]

I'm not sure if this will help you, just in case you still want to know how your work compiles on Windows with Cygwin.
I compiled your branch for you on Windows 8.1 (64-bit) + Cygwin, it compiles fine, there is only one warning:
function declaration isn’t a prototype [-Wstrict-prototypes] void Sys_InitProtocolHandler( );
This warning is related to the things zturtleman already mentioned.
Anyways, there are no errors or other warnings.

[stefansundin]

Okay, so I think I addressed all of the new feedback.

Thanks @KuehnhammerTobias. I was able to compile it on Windows but I don't recall seeing the warnings. I had so much pain getting it to work so I probably missed it.

To support IPv6 addresses, I simply added [ and ] to the allowed list of characters. I wrote this check of allowed characters to prevent attempts at issuing arbitrary commands, but I don't even know if Com_QueueEvent could be used to execute more than one command. Does anyone know if that check is strictly necessary? 🤷

I paginated through all the servers on https://www.quakeservers.net/quake3/servers/ but I couldn't see any IPv6 addresses there, so I haven't been able to test that it works as intended.

---

Another thing that is worth mentioning is that we've discussed this a bit in one of the Star Trek: Elite Force discords. It is very common to distribute mods using simple zip files, so without any installer that would perform the windows registry setup. This is less of a problem on macOS (just drag it to /Applications/ and macOS will discover the protocols in the plist) and Linux (you need a .desktop file but can't/shouldn't determine if that's already setup). There was a suggestion to add a console command that sets this up on Windows. What does everyone here think about that option?

Another option would be to include a .bat file that can be run to set it up, or a .reg file (worse option since the path can't be dynamically determined).

[zturtleman]

Arbitrary command execution

Using this pull request it's possible to run arbitrary console commands from clicking a link in a web browser. This has been tested on Linux. Windows has not been tested but is most likely also affected. macOS doesn't pass "quake3:…" on the command line so it is not affected.

This code has not been included in released software in my knowledge. It has been integrated into World of Padman source code by @mgerhardy though.

Main credit to @Daggolin for pointing out the arbitrary command execution issue.

Problem

The quake3:… token that is passed on the command line (by the web browser / operating system) is not removed from being processed as regular command line arguments. Command line arguments by default are all run as Quake 3 console commands and + is treated as a line-break in Quake 3 command line parsing.

As it is, the quake3:… token is added to arguments to execute in the console. This generates an unknown command "quake3:…" message and (unrelated to the function parsing the URI) allows arbitrary command execution.

• Linux shell: ioquake3.x86_64 quake3://+set pwned 1
• Linux shell using protocol handler: xdg-open "quake3://+set pwned 1"
• Web browser: <a href="quake3://+set/**/pwned/**/1"> open quake3 </a>

These launch the game and set pwned cvar to 1 but could run any series of commands. This can also be placed after connect URI command. quake3://connect/localhost?+set/**/pwned/**/1. Using cl_allowDownload 1 and a specified server address, this could potentially download a QVM exploit and have full access to your PC (with your user permissions).

When run from a web browser (tested on Linux with Firefox and Google Chrome) spaces and others that are whitespace in Q3 parser are URL encoded. Spaces in "set pwned 1" become %20 (the hex value of space) as quake3://connect/localhost?+set%20pwned%201 and ioquake3 tries to run the command set%20pwned%201 which fails as unknown command. However C comments /* comment */ act as a token delimiter allowing +set/**/pwned/**/1 to run the same as +set pwned 1.

Quote escape

This part is largely irrelevant since web browsers (Firefox and Google Chrome on Linux at least) turned out to replace spaces with %20 instead of passing quake3:… with spaces.

If quake3:… is passed as single argument with spaces to ioquake3, like in Linux terminal using double quotes or in software using exec(), ioquake3 would add double quotes around it before trying to execute it as a console command and treat it as a single argument. Inserting double quotes in the quake3:… token allows running arbitrary commands.

• Linux shell: ioquake3.x86_64 "quake3://connect/localhost?\"+set pwned 1+\""

Outer double quotes so Linux shell will pass it as a single argument to ioquake3 and escaped double quotes \" to be passed in the argument itself to ioquake3.

Solution

I would move checking for PROTOCOL_HANDLER to above Sys_ParseArgs( argc, argv ); and if protocol scheme is present set argc to 1 to disable processing quake3:… as a console command and any arguments after it, which might be possible if quake3: link has a space in it and software doesn't URL encode spaces as %20.

In code/sys/sys_main.c (pseudocode):

int main( int argc, char **argv ) {

...

	const char *uri = NULL;

...

#ifdef __APPLE__
	// This is passed if we are launched by double-clicking
	if ( argc >= 2 && Q_strncmp ( argv[1], "-psn", 4 ) == 0 )
		argc = 1;
#endif

	// This is passed if we are launched by protocol scheme handler
	if ( argc >= 2 && Q_strncmp ( argv[1], PROTOCOL_HANDLER ":", strlen( PROTOCOL_HANDLER ":" ) ) == 0 ) {
		uri = argv[1];
		argc = 1;
	}

	Sys_ParseArgs( argc, argv );

...

	where the URI is currently parsed:
	if ( uri ) {
		ParseUri( uri );
	}

Conclusion

I was going to test this issue on Windows and actually patch it but I haven't worked on this for almost two weeks now so I'm giving up.

I haven't thoroughly reviewed whether connect hostname in Com_QueueEvent() can be exploited. At face value the character limits in place by this PR seem like they're sufficient.

It would be a good idea to limit the hostname/address+port length to be less or equal to MAX_TOKEN_CHARS - 1 (1023 characters) so it doesn't split into multiple arguments in the parser. The way connect console command behaves—connect [-4|-6] <server> if there are two arguments the second is server name, otherwise first is server name—a link could hide the real server at 1024 characters into the hostname.

I think more security testing/review would probably be a good idea before shipping this to users. (I've run out of interest to work on it though.)

[stefansundin]

Ah, nice catch. There's also a slight risk if the user installs an ioq3 without protocol handler support (or a build where PROTOCOL_HANDLER is not configured), then registers support using a .reg file.

To make this more robust, a new switch can be added before the protocol URI itself, e.g. -link. Then the parsing can more easily skip the protocol string if the build doesn't support it, so it can skip the URI even if PROTOCOL_HANDLER is not defined.

Example: ioquake3.x86_64 -link quake3://+set pwned 1

The installer would write a slightly different registry key with -link before "%1":

WriteRegStr SHCTX "Software\Classes\quake3\shell\open\command" "" '"$INSTDIR\ioquake3.x86.exe" -link "%1"'

And to be safe it should just drop any extra arguments after the URI.

[7Saturn]

As a final note, I found it extremely hard to figure out how to compile and build on Windows. Even worse since the wiki is down. I eventually found it on archive.org (https://web.archive.org/web/20190506050817/http://wiki.ioquake3.org/Building_ioQuake3). I first tried to use Cygwin but failed. Then I tried msys2 and got it working but I had to mess with a lot of files. Lots that could be improved here. I should probably have abandoned my attempts and just relied on GitHub Actions for Windows builds. 🤷

It's similar for the Lilium Voyager project, which is derived from ioQuake3. Personally I cross-compile via Linux. Works a lot easier and the result does not disappoint. If only I could do that for MacOS target was well...

[stefansundin]

I just pushed a couple of commits. I believe this sufficiently addresses the security issue described above.

• The code checks for the protocol handler in the same loop where the "regular" cli arguments are processed. Which makes a lot more sense than what I originally tried.
  • I decided that my earlier suggestion of using a required argument before the quake3:// argument was good, since it makes it simple to discard the protocol string from the browser in case the game wasn't compiled with support for a protocol handler. It is not possible to know what the protocol handler would be without this, which makes it hard to ignore the argument. Now there's even less chance of misconfiguring a protocol handler in the registry and then using a build without PROTOCOL_HANDLER and running arbitrary console commands because of it. I decided to use --uri instead of -link, but I'm open to other suggestions of what to call this.
  • Instead of using Com_QueueEvent, the code basically just converts --uri quake3://connect/hostname:port into +connect hostname:port and appends it to commandLine, and the quake3:// string is not accidentally used elsewhere. The result is as if you used +connect hostname:port directly on the command line. macOS still uses Com_QueueEvent.
  • The quake3://connect/hostname:port argument no longer has to be the first one. So if you prefer that all quake 3 sessions that you launch from a browser is in windowed mode, you can update the registry key value to be "C:\ioq3\ioquake3.x86_64.exe" +r_fullscreen 0 --uri "%1" and it will work.
• Adds a \registerprotocolhandler command which writes the registry keys on Windows. This is useful for builds and mods that are distributed without an installer.
  

I tested these changes on macOS, Linux, and Windows.

[stefansundin]

This code now run too early for Z_malloc to work (I got segfault when I tried to use it).

[NuclearMonster]

• Adds a \registerprotocolhandler command which writes the registry keys on Windows. This is useful for builds and mods that are distributed without an installer.

Would this allow a malicious mod/game to write arbitrary commands to the registry or is that already protected by the other changes above?

[stefansundin]

Would this allow a malicious mod/game to write arbitrary commands to the registry or is that already protected by the other changes above?

You mean a random quake 3 mod that you downloaded somewhere on the internet? Since it is already executing code on your computer it wouldn't need \registerprotocolhandler to write to the registry. Only run executables from trusted sources. :)

By the way, is it possible for a server to run console commands on clients? Could a server initiate this command on a client's game? Even if that is the case I don't think it could be used particularly maliciously. The command doesn't take any arguments so it always writes the same registry keys based on what PROTOCOL_HANDLER was set to, which are the same as the installer would write:

  WriteRegStr SHCTX "Software\Classes\quake3" "CustomUrlApplication" "$INSTDIR\ioquake3.x86.exe"
  WriteRegStr SHCTX "Software\Classes\quake3" "CustomUrlArguments" '"%1"'
  WriteRegStr SHCTX "Software\Classes\quake3" "URL Protocol" ""
  WriteRegStr SHCTX "Software\Classes\quake3\DefaultIcon" "" "$INSTDIR\ioquake3.x86.exe,0"
  WriteRegStr SHCTX "Software\Classes\quake3\shell\open\command" "" '"$INSTDIR\ioquake3.x86.exe" --uri "%1"'

In the installer, SHCTX means that if you install for all users it will write to HKEY_LOCAL_MACHINE which enables the protocol handler for all users on the computer. If you install only for your user, then it uses HKEY_CURRENT_USER, and the protocol handler will only work for your user. I don't think an unprivileged user can write to HKEY_LOCAL_MACHINE, so I decided to use HKEY_CURRENT_USER in \registerprotocolhandler.

Hope this answered the question.

[stefansundin]

There is one problem on macOS that I am having a bit of a hard time solving..

For some reason, because of the AppDelegate or NSApp stuff that I added in sys_osx.m, there is a crash in/after Sys_Dialog runs. It is possible to test this by getting the "Abnormal Exit" prompt to run after a crash. The NSAlert shows up normally, but after clicking one of the buttons, the program crashes with this error:

2022-03-03 21:19:59.920 ioquake3.arm64[45354:113322] *** Assertion failure in -[NSApplication run], NSApplication.m:3367
2022-03-03 21:19:59.920 ioquake3.arm64[45354:113322] *** Terminating app due to uncaught exception 'NSInternalInconsistencyException', reason: 'NSApp with wrong _running count'
*** First throw call stack:
(
  0   CoreFoundation                      0x00000001834bc1cc __exceptionPreprocess + 240
  1   libobjc.A.dylib                     0x000000018320d7b8 objc_exception_throw + 60
  2   Foundation                          0x00000001843e3c84 -[NSCalendarDate initWithCoder:] + 0
  3   AppKit                              0x0000000185f81ae0 -[NSApplication run] + 896
  4   ioquake3.arm64                      0x00000001040c4b18 main + 540
  5   dyld                                0x0000000105eed0f4 start + 520
)
libc++abi: terminating with uncaught exception of type NSException
----- Client Shutdown (Received signal 6) -----

Deleting ~/Library/Application Support/Quake3/baseq3/ioq3.pid solves it since the "Abnormal Exit" prompt will not show, but this is just a workaround.

Commenting out the call to Sys_InitProtocolHandler prevents this error from happening, but this also disables the protocol handler feature from working. There must be a way to solve this but my macOS knowledge is limited. Does anyone have any idea? Maybe initialize the AppDelegate somewhere else?

[timangus]

What's the thinking behind the command in the first place? Not sure I understand the necessity if the installer already does it.

[stefansundin]

It was explained in this comment: #540 (comment)

Another thing that is worth mentioning is that we've discussed this a bit in one of the Star Trek: Elite Force discords. It is very common to distribute mods using simple zip files, so without any installer that would perform the windows registry setup. This is less of a problem on macOS (just drag it to /Applications/ and macOS will discover the protocols in the plist) and Linux (you need a .desktop file but can't/shouldn't determine if that's already setup). There was a suggestion to add a console command that sets this up on Windows. What does everyone here think about that option?

Another option would be to include a .bat file that can be run to set it up, or a .reg file (worse option since the path can't be dynamically determined).

I'm okay with removing it if that's a consensus that is reached here.

[timangus]

It seems like an odd thing to include, since it's solving a problem that exists outside of ioq3 and that could be solved by a simple .reg file as you say. (And I doubt people extracting things from zip files manually particularly care if the protocol handler registration ends up in HKLM or HKCU.)

Other than that it looks good.

[stefansundin]

Removed the console command.

Edit: I also rebased locally and made sure it works with all the changes merged since I forked. It seems to work just fine in my quick test. Let me know if you want me to force push the rebased branch. I am not able to force push this because it complains about the CI workflow being updated.

[timangus]

Is the Sys_*ExecutablePath stuff still necessary?

[stefansundin]

@timangus You're completely right, I forgot that it was related to the console command. Removed it.

I had a thought that calling this "protocol handler" might confuse it with the quake 3 network protocol. Perhaps it would be better to call it "os protocol handler" or "system protocol handler"? Let me know if I should rename it.

[timangus]

Hyperlink/link/url/uri handler would probably be more appropriate, and less open to misinterpretation as you say...

In any case you'll need to rebase onto main at some point since the Linux CI was apparently broken until recently, and won't complete if I do a test run here. What was the error you got when pushing?

[stefansundin]

I was able to do it now. I had accidentally configured my git remote to use https instead of SSH and this is the error I got:

To https://github.com/stefansundin/ioq3.git
 ! [remote rejected]   protocol-handler -> protocol-handler (refusing to allow a Personal Access Token to create or update workflow `.github/workflows/build.yml` without `workflow` scope)
error: failed to push some refs to 'https://github.com/stefansundin/ioq3.git'

I usually never use https to clone but I had used it to test something completely unrelated.