[FFmpeg-devel,1/2] avcodec/htmlsubtitles: Fixes denial of service due to use of sscanf in inner loop for tag scaning

Submitted by Michael Niedermayer on Feb. 13, 2019, 12:22 a.m.

Details

Message ID 20190213002238.32283-1-michael@niedermayer.cc
State New
Headers show

Commit Message

Michael Niedermayer Feb. 13, 2019, 12:22 a.m.
From: Kevin Backhouse via RT <security-reports@semmle.com>

Fixes: [Semmle Security Reports #19438]
Fixes: dos_sscanf1.mkv

Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/htmlsubtitles.c | 30 +++++++++++++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

Comments

Michael Niedermayer Feb. 17, 2019, 8:38 a.m.
On Wed, Feb 13, 2019 at 01:22:37AM +0100, Michael Niedermayer wrote:
> From: Kevin Backhouse via RT <security-reports@semmle.com>
> 
> Fixes: [Semmle Security Reports #19438]
> Fixes: dos_sscanf1.mkv
> 
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavcodec/htmlsubtitles.c | 30 +++++++++++++++++++++++++++++-
>  1 file changed, 29 insertions(+), 1 deletion(-)

will apply patchset

[...]

Patch hide | download patch | download mbox

diff --git a/libavcodec/htmlsubtitles.c b/libavcodec/htmlsubtitles.c
index fb9f900422..c0cfccfb16 100644
--- a/libavcodec/htmlsubtitles.c
+++ b/libavcodec/htmlsubtitles.c
@@ -74,6 +74,34 @@  struct font_tag {
     uint32_t color;
 };
 
+/*
+ * Fast code for scanning the rest of a tag. Functionally equivalent to
+ * this sscanf call:
+ *
+ * sscanf(in, "%127[^<>]>%n", buffer, lenp) == 2
+ */
+static int scantag(const char* in, char* buffer, int* lenp) {
+    int len;
+
+    for (len = 0; len < 128; len++) {
+        const char c = *in++;
+        switch (c) {
+        case '\0':
+            return 0;
+        case '<':
+            return 0;
+        case '>':
+            buffer[len] = '\0';
+            *lenp = len+1;
+            return 1;
+        default:
+            break;
+        }
+        buffer[len] = c;
+    }
+    return 0;
+}
+
 /*
  * The general politic of the convert is to mask unsupported tags or formatting
  * errors (but still alert the user/subtitles writer with an error/warning)
@@ -155,7 +183,7 @@  int ff_htmlmarkup_to_ass(void *log_ctx, AVBPrint *dst, const char *in)
 
             len = 0;
 
-            if (sscanf(in+tag_close+1, "%127[^<>]>%n", buffer, &len) >= 1 && len > 0) {
+            if (scantag(in+tag_close+1, buffer, &len) && len > 0) {
                 const int skip = len + tag_close;
                 const char *tagname = buffer;
                 while (*tagname == ' ') {