Buffer overflow while querying Quake1 server rules

[illwieckz]

Hi, I'm experiencing a buffer overflow when I try to query some Quake1 servers while asking for server rules.

Basic query (works):

$ qstat -qs 109.228.169.24:26003
ADDRESS           PLAYERS      MAP   RESPONSE TIME    NAME
109.228.169.24:26003   0/8   0/0     intro    121 / 0   QRF_COOP

Server rules query (do not work):

qstat -R -qs 109.228.169.24:26003
ADDRESS           PLAYERS      MAP   RESPONSE TIME    NAME
*** buffer overflow detected ***: qstat terminated
======= Backtrace: =========
/lib/x86_64-linux-gnu/libc.so.6(+0x78c4e)[0x7f47ea648c4e]
/lib/x86_64-linux-gnu/libc.so.6(__fortify_fail+0x5c)[0x7f47ea6e8e8c]
/lib/x86_64-linux-gnu/libc.so.6(+0x116e80)[0x7f47ea6e6e80]
qstat[0x40ceae]
qstat[0x40e97a]
qstat[0x40eb1d]
qstat[0x41f6ab]
qstat[0x402333]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf0)[0x7f47ea5f0a40]
qstat[0x403829]
======= Memory map: ========
00400000-00445000 r-xp 00000000 fc:02 219994                             /usr/bin/qstat
00645000-00646000 r--p 00045000 fc:02 219994                             /usr/bin/qstat
00646000-0064c000 rw-p 00046000 fc:02 219994                             /usr/bin/qstat
0064c000-0065a000 rw-p 00000000 00:00 0 
00a82000-00aa3000 rw-p 00000000 00:00 0                                  [heap]
7f47ea138000-7f47ea14e000 r-xp 00000000 fc:02 6030633                    /lib/x86_64-linux-gnu/libgcc_s.so.1
7f47ea14e000-7f47ea34d000 ---p 00016000 fc:02 6030633                    /lib/x86_64-linux-gnu/libgcc_s.so.1
7f47ea34d000-7f47ea34e000 rw-p 00015000 fc:02 6030633                    /lib/x86_64-linux-gnu/libgcc_s.so.1
7f47ea34f000-7f47ea5d0000 rw-p 00000000 00:00 0 
7f47ea5d0000-7f47ea790000 r-xp 00000000 fc:02 6030624                    /lib/x86_64-linux-gnu/libc-2.21.so
7f47ea790000-7f47ea990000 ---p 001c0000 fc:02 6030624                    /lib/x86_64-linux-gnu/libc-2.21.so
7f47ea990000-7f47ea994000 r--p 001c0000 fc:02 6030624                    /lib/x86_64-linux-gnu/libc-2.21.so
7f47ea994000-7f47ea996000 rw-p 001c4000 fc:02 6030624                    /lib/x86_64-linux-gnu/libc-2.21.so
7f47ea996000-7f47ea99a000 rw-p 00000000 00:00 0 
7f47ea9a0000-7f47ea9c4000 r-xp 00000000 fc:02 6030594                    /lib/x86_64-linux-gnu/ld-2.21.so
7f47eabbf000-7f47eabc3000 rw-p 00000000 00:00 0 
7f47eabc3000-7f47eabc4000 r--p 00023000 fc:02 6030594                    /lib/x86_64-linux-gnu/ld-2.21.so
7f47eabc4000-7f47eabc5000 rw-p 00024000 fc:02 6030594                    /lib/x86_64-linux-gnu/ld-2.21.so
7f47eabc5000-7f47eabc7000 rw-p 00000000 00:00 0 
7f47eabc7000-7f47eabc9000 rw-p 00000000 00:00 0 
7ffe87c75000-7ffe87c96000 rw-p 00000000 00:00 0                          [stack]
7ffe87da8000-7ffe87daa000 r--p 00000000 00:00 0                          [vvar]
7ffe87daa000-7ffe87dac000 r-xp 00000000 00:00 0                          [vdso]
ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0                  [vsyscall]
Abandon

It fails while trying an strcpy in send_rule_request_packet function in qstat.c.

You can experiment with server 109.228.169.24:26003 but probably anyone from this list:

• http://www.qtracker.com/server_list_details.php?game=quake

This was tested on the last revision of the master branch.

Yeah, the code has sprintf(), strcpy(), and strcat() in it.. those can probably be converted over to snprintf(), strlcopy(), and strlcat() at some point.

I'll look into it and put a PR in. 👍
(As I get warnings when I compile about those functions on OpenBSD)

[stevenh]

there are some older methods which could indeed lead to issues, PRs welcome

@stevenh @illwieckz - I've made a start converting sprintf to snprintf and the other functions as well. I've not quite gotten all of them converted over yet, however I ran a while loop through that list from qtracker.com and I didn't get any crashes this time. Once I've updated the functions, I'll send a PR.

[illwieckz]

nice!!

@illwieckz @stevenh According to the struct for q_rule, the 'data' element is: unsigned char data[19];
So that's only 19 characters allocated for rules, so any rules over that length will overflow.
If that strcpy gets converted to a strlcpy, then you will only see the first 19 chars of the rules is my guess. If you wanted more, than q_packet struct would need changing I think. (With a longer data[] length)

[stevenh]

No sure what you mean by q_rule as thats only used in one place, if you mean rules in general then they are dealt with differently.

Hi @illwieckz - Are you able to reproduce this at all with the latest version? I attempted to do so myself and used the following one-liner:

for srv in $(qstat -qwm master.quakeservers.net:27000 | awk '{ print $2}'); do qstat -qws ${srv}; done

(You can use any of the master quakeworld servers here: https://www.quakeservers.net/quakeworld/master_servers/

I wasn't able to reproduce this at all.