Introduction

In CVE-2018-13379, untrusted user could use the feature of snprintf() to launch the attack of arbitrary file reading.
Isn’t snprintf() already a safer function?
What kind of the feature can be used to bypass the limitation of file extension?
In this article, I would not only analyze the way of launching an attack, but also share some ideas of mitigation!

The feature of vulnerable code

snprintf(s, 0x40, "/migadmin/lang/%s.json", lang);

The code snippet above is the vulnerable part of source code (reversed by security researchers). In this part, I would introduce the function of snprintf() and how it works in the system. The function snprintf() would compose a string with specified format and put it into the buffer. Take the code snippet above for example, the content of variable lang would be concatenated between string of “/migadmin/lang/” and “.json” as string format. After concatenation, the whole string would be put into the buffer pointed by s on the first argument. What’s more important, if the length of string is longer than (0x40 - 1), the remaining characters would be discarded. In function snprintf(), terminating null character would always be appended to the end of string in buffer automatically (this is good!), and this is also the reason that limitation of length needs to minus one additionally.
In the vulnerable system, users can set up their custom language with adding file names. The code above shows how the guest adds the language. The system would be able to read the language json files in the buffer.

The analysis of vulnerability

In perspective of buffer overflow, snprintf() is a safer function than strcpy() or strcat() because it has limitation of length and a gurantee of terminated character at the end of string. Although it still has problem such like inefficiency because snprintf() would continue reading to the end of input to determine the return value(if there is no \0 in input…), it can be fixed with precision field to set up the maximum length of source string. The truly serious problem in this case is arbitrary file reading.
In fact, the vulnerabilities do not exist in the function snprintf() itself, but exist in the way by which the developers use the function. First, they concatenate the string with untrusted user input directly. Second, there is even no any filters of “../”. If the file path can be controlled by the untrusted user, then the priority should be preventing user from inputting slash and dot. With multiple groups of “../”, untrusted user would be able to read arbitrary files in parent directory without permission, which is called file reading with path traversal.
However, it is still not called arbitrary file reading because developers have limited the file extension to be json. Even if the untrusted users get into the parent directory, they could only read the file with extension of json. Here, the security researcher came up with an interesting idea: utilization of snprintf’s feature. In the previous section, we already know that function snprintf() would discard the remaining characters if the length of resulting string is longer than (limited-length - 1). The resulting string also includes “.json” at the end; therefore, we could discard the “.json” characters by making them become the remaining characters.

/migadmin/lang//../../../..//////////////////////////////bin/sh.json

The payload above could be used to read the file of “/bin/sh” without permission. The length of string before “.json” is 63, which is (0x40 - 1). Therefore, function snprintf() would discard the remaining characters, which are “.json”, and append the terminated character to the buffer. With this trick, untrusted user can do arbitrary file reading on system files.

Mitigation

In this section, I would provide the way to mitigate the vulnerability of arbitrary file reading.

const regex re("^[A-Za-z0-9\-\_\~]+$");
if(regex_match(lang, re)){
    int ret = 0;
    do {
        ret = snprintf(s, 0x40, "/migadmin/lang/%s.json", lang);
        // snprintf(s, 0x40, "/migadmin/lang/%.*s.json", 0x40 - 0xf - 0x5 - 0x1, lang);
    }while( ret<0 || ret>=0x40 );
}else{
    printf("You filename includes some illegal characters!!\n");
}

Here I share two ideas of mitigation. The key point is the the return value of snprintf. The biggest difference between sprintf() and snprintf() is their return value. sprintf() would return the number of successfully overwritten string; while snprintf() would return the number of intended written string, which means the whole string you want to put into the buffer!
Developers should never underestimate the use of snprintf’s return value because it can be used to check error and string truncation! If return value is less than 0, it means that there is an error with the function; if return value is bigger or equal to buffer size, it means that it should be at least one character discarded(one character discarded due to \0 at the end of buffer).
Another key point is precision field that you can specify the maximum length of input with parameter. This can handle the inefficiency issue mentioned above, function would stop reading after reaching specified maximum length!

Reference

• “SNPRINTF.” CPLUSPLUS.COM
• CVE-2018-13379
• ATTACKING SSL VPN - PART 2: BREAKING THE FORTIGATE SSL VPN