Spotting Silent Patches in OSS with Binary Static Analysis

In this blog post, we want to explore an interesting by-product of our recently released binary static analysis feature: identifying silently patched vulnerabilities.

Even if some may think otherwise, not every vulnerability is assigned a CVE. Sometimes, open-source project maintainers will fix a null dereference or buffer overflow and go on with their day. We should not expect them to file a CVE for it. However, this silent patching will not alert manufacturers using the affected software component. Unaware of security concerns, they might opt to stick with an older version. As a result, due to the absence of a CVE associated with this bug fix, your device could remain vulnerable without your knowledge, even if you have a robust vulnerability management process in place.

So here’s a small anthology of bugs spotted by binary static analysis in open-source libraries found in the firmware of connected devices, which are still being sold in 2024. We will keep updating the list throughout the year as some of our discoveries are still missing the “patch” part of “silent patch”.

FreeRadius rc_read_dictionary Stack Buffer Overflow

FreeRadius uses dictionary files:

The master RADIUS dictionary file resides in /etc/raddb/dictionary. It references other dictionary files located in /usr/local/share/freeradius/. Each dictionary file contains a list of RADIUS attributes and values, which the server uses to map between descriptive names and on-the-wire data. The names have no meaning outside of the RADIUS server itself, and are never exchanged between server and clients.

FreeRADIUS ‘man’ pages

These files are read through the rc_read_dictionary function, which must be called prior to any radius activity, as can be seen here:

./src/radstatus.c:	if (rc_read_dictionary(rh, rc_conf_str(rh, "dictionary")) != 0)
./src/radacct.c:	if (rc_read_dictionary(rh, rc_conf_str(rh, "dictionary")) != 0)
./src/radlogin.c:	if (rc_read_dictionary(rh, rc_conf_str(rh, "dictionary")) != 0)
./src/radexample.c:	if (rc_read_dictionary(rh, rc_conf_str(rh, "dictionary")) != 0)
./src/radiusclient.c:    if (rc_read_dictionary(rh, rc_conf_str(rh, "dictionary")) != 0) {
./src/radembedded.c:	if (rc_read_dictionary(rh, rc_conf_str(rh, "dictionary")) != 0) 

Binary static analysis spotted a bug in the rc_read_dictionary implementation where an unsafe call to sscanf could lead to stack buffer overflows. This is due to the fact that sscanf will perform unbounded copies unless the length is explicitly provided in the format string (e.g. %63s rather than %s).

It got silently fixed in 2014 with commit d527424f4cb71057e5d468d88864dc054508418b. The commit message says “fixed memleaks and etc.” 🙂

Here’s the diff:

diff --git a/lib/dict.c b/lib/dict.c
index 0062df3..0a5aa36 100644
--- a/lib/dict.c
+++ b/lib/dict.c
@@ -75,7 +75,7 @@ int rc_read_dictionary (rc_handle *rh, char const *filename)
                {
                        optstr[0] = '\0';
                        /* Read the ATTRIBUTE line */
-                       if (sscanf (buffer, "%s%s%s%s%s", dummystr, namestr,
+                       if (sscanf (buffer, "%63s%63s%63s%63s%63s", dummystr, namestr,
                                    valstr, typestr, optstr) < 4)
                        {
                                rc_log(LOG_ERR, "rc_read_dictionary: invalid attribute on line %d of dictionary %s",
@@ -173,7 +173,7 @@ int rc_read_dictionary (rc_handle *rh, char const *filename)
                else if (strncmp (buffer, "VALUE", 5) == 0)
                {
                        /* Read the VALUE line */
-                       if (sscanf (buffer, "%s%s%s%s", dummystr, attrstr,
+                       if (sscanf (buffer, "%63s%63s%63s%63s", dummystr, attrstr,
                                    namestr, valstr) != 4)
                        {
                                rc_log(LOG_ERR,
@@ -232,7 +232,7 @@ int rc_read_dictionary (rc_handle *rh, char const *filename)
                 else if (strncmp (buffer, "$INCLUDE", 8) == 0)
                 {
                        /* Read the $INCLUDE line */
-                       if (sscanf (buffer, "%s%s", dummystr, namestr) != 2)
+                       if (sscanf (buffer, "%63s%63s", dummystr, namestr) != 2)
                        {
                                rc_log(LOG_ERR,
                                 "rc_read_dictionary: invalid include entry on line %d of dictionary %s",
@@ -247,7 +247,7 @@ int rc_read_dictionary (rc_handle *rh, char const *filename)
                                if (cp != NULL) {
                                        ifilename = alloca(AUTH_ID_LEN);
                                        *cp = '\0';
-                                       sprintf(ifilename, "%s/%s", filename, namestr);
+                                       snprintf(ifilename, AUTH_ID_LEN, "%s/%s", filename, namestr);
                                        *cp = "https://onekey.com/";
                                }
                        }
@@ -260,7 +260,7 @@ int rc_read_dictionary (rc_handle *rh, char const *filename)
                else if (strncmp (buffer, "VENDOR", 6) == 0)
                {
                        /* Read the VALUE line */
-                       if (sscanf (buffer, "%s%s%s", dummystr, attrstr, valstr) != 3)
+                       if (sscanf (buffer, "%63s%63s%63s", dummystr, attrstr, valstr) != 3)
                        {
                                rc_log(LOG_ERR,
                                 "rc_read_dictionary: invalid Vendor-Id on line %d of dictionary %s",

The buffers in which sscanf writes are AUTH_ID_LEN (64) bytes long. So by manipulating the dictionary file or by pointing radius to a different dictionary file, it’s possible to overflow into adjacent stack memory.

Gaining arbitrary code execution however seems unlikely due to the stack layout and variables stored there.

NetSNMP read_config_store Stack Buffer Overflow

The environment variable SNMP_PERSISTENT_FILE is used by Net-SNMP to know from where it should read its configuration:

This environment variable is then used as a parameter when calling mkdirhier:

Finally, the environment variable is appended to a stack buffer of 4096 bytes using strcat:

Thanks to git blame and git bisect we were able to identify the patch (Get rid of (insecure) SNMP_MAXPATH in mkdirhier() · net-snmp/net-snmp@8850616). While they keep using strcat, they append to a buffer allocated on the heap, with a size that corresponds to the user controlled data. So it’s no longer exploitable.

This fix dates back to January 2012 and the vulnerable implementation is still observed in embedded devices sold in 2024. Of course, the vendor should have updated this library way before that. The buffer overflow was picked up in net-snmp 5.1.1, for which the platform already identified good reasons to update:

Key Takeaways

While most security processes only rely on public vulnerability databases such as NVD for vulnerability management and due-diligence of software dependencies, the aforementioned issues highlight the potential for overlooking vulnerabilities within your supply chain relying on this method only.

In the wake of the upcoming European Cyber Resilience Act with increased liability and hefty fines, modern vulnerability management programs need to enhance their diligence in assessing software dependencies. Mere reliance on CVEs will no longer suffice. A more thorough analysis of all dependencies will be required to demonstrate conformity. Moreover, if you identify vulnerabilities that have not yet been (silently or loudly) fixed, don’t forget to share your security fixes with the maintainers of your dependencies so that the entire ecosystem can benefit from them. After all, this proactive sharing will also be mandated by the Cyber Resilience Act.