Astute Explorer (GCHQ Challenge 1 – 5)

GCHQ has been having trouble finding experienced hackers and programmers to work for them, so they’ve put out a lot of fun challenges. The idea is that people who do well in the online challenges are selected to do face to face challenges, the top few people from the face to face challenge go through to the masterclass, then the top people from the masterclass will be vetted for a job. Sadly I missed the first challenge, but managed to get into “Astute Explorer” just in time (It’s finished now so I’m posting my answers). The challenge is an imaginary company is under cyber attack and want you to help secure their software, they have even been kind enough to provided you with random snippets of C code (without any context). Your job is to provide the line number of the vulnerability, explain why it’s a vulnerability, and how to patch it. So let’s take a look (my area of expertise is malware, so sorry if I end up getting stuff wrong).

Vulnerability
I’m not sure there’s an exact line number, but the vulnerability here is pretty obvious. strcpy and strcat don’t do any checks to make sure the target buffer is large enough to fit the string, so by providing a username/password that exceeds the size of szTotalString, you can cause a buffer overflow.

Solution
A lot of people would use functions like strlcpy/StringCchCopy, which copy as much data into the buffer as it can fit then null terminates it. In my opinion this is stupid because the code would still try to check the username and password despite the fact not all of it was copied, I’d personally do strlen on the username and password, then fail if it exceeds the maximum length.

Vulnerability
This is a fun one, you can chain multiple vulnerabilities in the code to cause a heap overflow. The height and width are user specified, so the first thought would be to specify a height and width so large that it causes an integer overflow when calculating the size on line 94, resulting in the allocated buffer being too small to accommodate board_squared_t. If you look at line 91 it makes sure that the height and width don’t exceed a certain size, Foiled! Or not? The mistake here is that height and width are signed integers and if you listened in maths: multiplying 2 negative values gives a positive result. We can bypass the maximum size check by specifying large negative values instead, which will then become positive and still cause our integer overflow.

Solution
Pretty simple, just declare height and width as unsigned integers instead of signed.

Vulnerability
I literally have no idea what’s going on here. Why is iSize being multiplied by the size of int (4)? What is szBuffer? What does the number is %d = %d mean? If someone handed me this code I’d assume they got their degree in a cereal box and send them back to work at geek squad. Assuming pszArguments[2] is a string and pszArguments[1] is the length of the string, the memcpy operation on line 556 is going to copy 4x more bytes that the length of the string and probably crash. It should also be noted that iArguments isn’t checked, so if the user doesn’t specify enough arguments, the application is going to crash.

Solution
Stop doing drugs at work.

Vulnerability
On line 84 szError is passed as the format argument to printf. Because printf will interpret any format specifiers in the format argument this is unsafe (format strings exploit). If the error string were to contain “%d%d%d” the next 12 bytes of memory on the stack would be output to the user, it’s also possible to use the %n specifier to write arbitrary data to an arbitrary location.

Solution
printf(“%s”, szError);

Vulnerability
I wasn’t able to find any major vulnerabilities here. The code leaks memory because _strdup allocates a buffer, which is never freed. There’s also an off-by-one error on line 352: the code checks if the filename is larger than or equal to 3, however the extensions in the szOkExt array include the dot (4 bytes), if the user specified a 3 byte filename the code would try to compare 4 bytes and possibly (but unlikely) crash the application.

Solution
Check if cFileName is bigger than 3 and free szLCase before return.