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.

[tdm4]

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

[tdm4]

@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!!

[tdm4]

@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.

[tdm4]

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.