CVE-2021-30145: Use of Externally-Controlled Format String (CWE-134)
value: A format string vulnerability in mpv through 0.33.0 allows user-assisted remote attackers to achieve code execution via a crafted m3u playlist file.
CWE description: The product uses a function that accepts a format string as an argument, but the format string originates from an external source.
CVSS v3 base score: 7.8
Published: 2021-05-18T14:15Z
Repository: https://github.com/mpv-player/mpv
Language: C
Files changed: demux/demux_mf.c
Commit date: 2021-04-05 18:24:55 +0300
Commit message: demux_mf: improve format string processing

Before this commit, the user could specify a printf format string
which wasn't verified, and could result in:
- Undefined behavior due to missing or non-matching arguments.
- Buffer overflow due to untested result length.

The offending code was added at commit 103a9609 (2002, mplayer svn):
git-svn-id: svn://svn.mplayerhq.hu/mplayer/trunk@4566 b3059339-0415-0410-9bf9-f77b7e298cf2

It moved around but was not modified meaningfully until now.

Now we reject all conversion specifiers at the format except %%
and a simple subset of the valid specifiers. Also, we now use
snprintf to avoid buffer overflow.

The format string is provided by the user as part of mf:// URI.

Report and initial patch by Stefan Schiller.
Patch reviewed by @jeeb, @sfan5, Stefan Schiller.

Diff: diff --git a/demux/demux_mf.c b/demux/demux_mf.c
index 424821b965..40f94f4e4e 100644
--- a/demux/demux_mf.c
+++ b/demux/demux_mf.c
@@ -119,11 +119,12 @@ static mf_t *open_mf_pattern(void *talloc_ctx, struct demuxer *d, char *filename
         mp_info(log, "number of files: %d\n", mf->nr_of_files);
 
         goto exit_mf;
     }
 
-    char *fname = talloc_size(mf, strlen(filename) + 32);
+    size_t fname_avail = strlen(filename) + 32;
+    char *fname = talloc_size(mf, fname_avail);
 
 #if HAVE_GLOB
     if (!strchr(filename, '%')) {
         strcpy(fname, filename);
         if (!strchr(filename, '*'))
@@ -146,14 +147,48 @@ static mf_t *open_mf_pattern(void *talloc_ctx, struct demuxer *d, char *filename
         globfree(&gg);
         goto exit_mf;
     }
 #endif
 
+    // We're using arbitrary user input as printf format with 1 int argument.
+    // Any format which uses exactly 1 int argument would be valid, but for
+    // simplicity we reject all conversion specifiers except %% and simple
+    // integer specifier: %[.][NUM]d where NUM is 1-3 digits (%.d is valid)
+    const char *f = filename;
+    int MAXDIGS = 3, nspec = 0, bad_spec = 0, c;
+
+    while (nspec < 2 && (c = *f++)) {
+        if (c != '%')
+            continue;
+        if (*f != '%') {
+            nspec++;  // conversion specifier which isn't %%
+            if (*f == '.')
+                f++;
+            for (int ndig = 0; mp_isdigit(*f) && ndig < MAXDIGS; ndig++, f++)
+                /* no-op */;
+            if (*f != 'd') {
+                bad_spec++;  // not int, or beyond our validation capacity
+                break;
+            }
+        }
+        // *f is '%' or 'd'
+        f++;
+    }
+
+    // nspec==0 (zero specifiers) is rejected because fname wouldn't advance.
+    if (bad_spec || nspec != 1) {
+        mp_err(log, "unsupported expr format: '%s'\n", filename);
+        goto exit_mf;
+    }
+
     mp_info(log, "search expr: %s\n", filename);
 
     while (error_count < 5) {
-        sprintf(fname, filename, count++);
+        if (snprintf(fname, fname_avail, filename, count++) >= fname_avail) {
+            mp_err(log, "format result too long: '%s'\n", filename);
+            goto exit_mf;
+        }
         if (!mp_path_exists(fname)) {
             error_count++;
             mp_verbose(log, "file not found: '%s'\n", fname);
         } else {
             mf_add(mf, fname);

Vulnerable code:     char *fname = talloc_size(mf, strlen(filename) + 32);
        sprintf(fname, filename, count++);
Fixed code:     size_t fname_avail = strlen(filename) + 32;
    char *fname = talloc_size(mf, fname_avail);
    // We're using arbitrary user input as printf format with 1 int argument.
    // Any format which uses exactly 1 int argument would be valid, but for
    // simplicity we reject all conversion specifiers except %% and simple
    // integer specifier: %[.][NUM]d where NUM is 1-3 digits (%.d is valid)
    const char *f = filename;
    int MAXDIGS = 3, nspec = 0, bad_spec = 0, c;

    while (nspec < 2 && (c = *f++)) {
        if (c != '%')
            continue;
        if (*f != '%') {
            nspec++;  // conversion specifier which isn't %%
            if (*f == '.')
                f++;
            for (int ndig = 0; mp_isdigit(*f) && ndig < MAXDIGS; ndig++, f++)
                /* no-op */;
            if (*f != 'd') {
                bad_spec++;  // not int, or beyond our validation capacity
                break;
            }
        }
        // *f is '%' or 'd'
        f++;
    }

    // nspec==0 (zero specifiers) is rejected because fname wouldn't advance.
    if (bad_spec || nspec != 1) {
        mp_err(log, "unsupported expr format: '%s'\n", filename);
        goto exit_mf;
    }

        if (snprintf(fname, fname_avail, filename, count++) >= fname_avail) {
            mp_err(log, "format result too long: '%s'\n", filename);
            goto exit_mf;
        }