[PR] avformat/http: reject malformed response status lines (PR #24232)

PR #24232 opened by Romain Beauxis (toots)
URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24232
Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24232.patch

This PR fixes issues with native HTTP parser accepting malformed response status lines.

These issues were uncovered while working on adding `ICY` support to the new `libcurl` implementation.

Before the changes, malformed HTTP status responses were mistakenly taken for valid `200` responses, prompting the parser to accept them and proceed further.

One side-effect was that `ICY 200` requests were accepted (as well as `ICY 404` requests). The changes in this patchset remove this unexpected feature.

Worth noting: `ICY` status response is very old and specific to shoutcast so I don't think that we should worry too much about support them.

Opt-in support for them is planned as part of the upcoming changes to the new `libcurl` backend.

See https://code.ffmpeg.org/toots/FFmpeg/commit/2cfcd0c11dc08b9d8a58c6962e71af69af68ff42 for tests details.


>From 9b76690f4926fcd81a012b49bd161308f244f8a1 Mon Sep 17 00:00:00 2001
From: Romain Beauxis <[email protected]>
Date: Fri, 21 Aug 2026 08:27:16 -0500
Subject: [PATCH 1/3] avformat/http: split the response status line parser out
 of process_line

Makes the parser reachable from a test program.  No functional change, beyond
checking the allocation of the exported version string.
---
 libavformat/http.c | 59 +++++++++++++++++++++++++++++++++-------------
 libavformat/http.h | 21 +++++++++++++++++
 2 files changed, 64 insertions(+), 16 deletions(-)

diff --git a/libavformat/http.c b/libavformat/http.c
index 69d4c8ab72..50fd88970d 100644
--- a/libavformat/http.c
+++ b/libavformat/http.c
@@ -890,6 +890,34 @@ static int http_get_line(HTTPContext *s, char *line, int line_size)
     }
 }
 
+int ff_http_parse_status_line(void *logctx, const char *line, HTTPStatusLine *st)
+{
+    const char *p = line;
+    char *end;
+
+    memset(st, 0, sizeof(*st));
+
+    if (av_strncasecmp(p, "HTTP/1.0", 8) == 0)
+        st->willclose = 1;
+
+    while (*p != '/' && *p != '\0')
+        p++;
+    while (*p == '/')
+        p++;
+    av_strlcpy(st->version, p, sizeof(st->version));
+
+    while (!av_isspace(*p) && *p != '\0')
+        p++;
+    while (av_isspace(*p))
+        p++;
+    st->code   = strtol(p, &end, 10);
+    st->reason = end;
+
+    av_log(logctx, AV_LOG_TRACE, "http_code=%d\n", st->code);
+
+    return 0;
+}
+
 static int check_http_code(URLContext *h, int http_code, const char *end)
 {
     HTTPContext *s = h->priv_data;
@@ -1181,7 +1209,7 @@ static int process_line(URLContext *h, char *line, int line_count, int *parsed_h
 {
     HTTPContext *s = h->priv_data;
     const char *auto_method =  h->flags & AVIO_FLAG_READ ? "POST" : "GET";
-    char *tag, *p, *end, *method, *resource, *version;
+    char *tag, *p, *method, *resource, *version;
     int ret;
 
     /* end of header */
@@ -1245,25 +1273,24 @@ static int process_line(URLContext *h, char *line, int line_count, int *parsed_h
             }
             av_log(h, AV_LOG_TRACE, "HTTP version string: %s\n", version);
         } else {
-            if (av_strncasecmp(p, "HTTP/1.0", 8) == 0)
-                s->willclose = 1;
-            while (*p != '/' && *p != '\0')
-                p++;
-            while (*p == '/')
-                p++;
-            av_freep(&s->http_version);
-            s->http_version = av_strndup(p, 3);
-            while (!av_isspace(*p) && *p != '\0')
-                p++;
-            while (av_isspace(*p))
-                p++;
-            s->http_code = strtol(p, &end, 10);
+            HTTPStatusLine st;
 
-            av_log(h, AV_LOG_TRACE, "http_code=%d\n", s->http_code);
+            if ((ret = ff_http_parse_status_line(h, p, &st)) < 0)
+                return ret;
+
+            /* Only ever set: a keep-alive decision made earlier must survive. */
+            if (st.willclose)
+                s->willclose = 1;
+
+            av_freep(&s->http_version);
+            if (!(s->http_version = av_strdup(st.version)))
+                return AVERROR(ENOMEM);
+
+            s->http_code = st.code;
 
             *parsed_http_code = 1;
 
-            if ((ret = check_http_code(h, s->http_code, end)) < 0)
+            if ((ret = check_http_code(h, s->http_code, st.reason)) < 0)
                 return ret;
         }
     } else {
diff --git a/libavformat/http.h b/libavformat/http.h
index 08605c8f91..dd2c15789e 100644
--- a/libavformat/http.h
+++ b/libavformat/http.h
@@ -27,6 +27,27 @@
 
 #define HTTP_HEADERS_SIZE 4096
 
+/**
+ * Parsed form of a response status line.
+ */
+typedef struct HTTPStatusLine {
+    char        version[4]; /**< protocol version, e.g. "1.1" */
+    int         code;       /**< response status code */
+    const char *reason;     /**< reason phrase, points into the parsed line */
+    int         willclose;  /**< the version implies the connection will close */
+} HTTPStatusLine;
+
+/**
+ * Parse the status line of an HTTP response.
+ *
+ * @param logctx context used for logging, may be NULL
+ * @param line NUL terminated status line, without its trailing CRLF
+ * @param st filled in with the parsed status line
+ * @return 0 on success, a negative AVERROR code on failure
+ */
+int ff_http_parse_status_line(void *logctx, const char *line,
+                              HTTPStatusLine *st);
+
 /**
  * Initialize the authentication state based on another HTTP URLContext.
  * This can be used to pre-initialize the authentication parameters if
-- 
2.52.0


>From 2cfcd0c11dc08b9d8a58c6962e71af69af68ff42 Mon Sep 17 00:00:00 2001
From: Romain Beauxis <[email protected]>
Date: Fri, 21 Aug 2026 08:27:25 -0500
Subject: [PATCH 2/3] tests/http: test response status line parsing

Covers well formed status lines, a missing or non numeric status code, and
lines that do not start with an HTTP version.
---
 libavformat/Makefile            |  1 +
 libavformat/tests/.gitignore    |  1 +
 libavformat/tests/http.c        | 67 +++++++++++++++++++++++++++++++++
 tests/fate/libavformat.mak      |  4 ++
 tests/ref/fate/http-status-line | 28 ++++++++++++++
 5 files changed, 101 insertions(+)
 create mode 100644 libavformat/tests/http.c
 create mode 100644 tests/ref/fate/http-status-line

diff --git a/libavformat/Makefile b/libavformat/Makefile
index 466f5d1894..038e0afd41 100644
--- a/libavformat/Makefile
+++ b/libavformat/Makefile
@@ -790,6 +790,7 @@ TESTPROGS = id3v2                                                       \
 FIFO-MUXER-TESTPROGS-$(CONFIG_NETWORK)   += fifo_muxer
 TESTPROGS-$(CONFIG_FIFO_MUXER)           += $(FIFO-MUXER-TESTPROGS-yes)
 TESTPROGS-$(CONFIG_FFRTMPCRYPT_PROTOCOL) += rtmpdh
+TESTPROGS-$(CONFIG_HTTP_PROTOCOL)        += http
 TESTPROGS-$(CONFIG_NETWORK)              += noproxy
 TESTPROGS-$(CONFIG_SRTP)                 += srtp
 TESTPROGS-$(CONFIG_IMF_DEMUXER)          += imf
diff --git a/libavformat/tests/.gitignore b/libavformat/tests/.gitignore
index 1807488603..2be3cc8fac 100644
--- a/libavformat/tests/.gitignore
+++ b/libavformat/tests/.gitignore
@@ -9,3 +9,4 @@
 /srtp
 /url
 /seek_utils
+/http
diff --git a/libavformat/tests/http.c b/libavformat/tests/http.c
new file mode 100644
index 0000000000..3767cf319b
--- /dev/null
+++ b/libavformat/tests/http.c
@@ -0,0 +1,67 @@
+/*
+ * Copyright (c) 2026 Romain Beauxis
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include <stdio.h>
+
+#include "libavformat/http.h"
+
+static void test(const char *line)
+{
+    HTTPStatusLine st;
+    int ret;
+
+    printf("\"%s\"\n", line);
+
+    ret = ff_http_parse_status_line(NULL, line, &st);
+    if (ret < 0) {
+        printf("  rejected\n");
+        return;
+    }
+
+    printf("  version=\"%s\" code=%d willclose=%d reason=\"%s\"\n",
+           st.version, st.code, st.willclose, st.reason);
+}
+
+int main(void)
+{
+    /* Well formed. */
+    test("HTTP/1.1 200 OK");
+    test("HTTP/1.0 200 OK");
+    test("HTTP/1.1 404 Not Found");
+    test("HTTP/1.1 500 Internal Server Error");
+    test("HTTP/1.1 204 No Content");
+
+    /* Well formed, without a reason phrase. */
+    test("HTTP/1.1 200");
+
+    /* Malformed: no version, no status code, or not HTTP at all. */
+    test("HTTP/1.1");
+    test("HTTP/1.1 OK");
+    test("HTTP");
+    test("200 OK");
+    test("NOT A STATUS LINE");
+    test("");
+
+    /* Shoutcast answers with its own status line, which is not HTTP. */
+    test("ICY 200 OK");
+    test("ICY 404 Not Found");
+
+    return 0;
+}
diff --git a/tests/fate/libavformat.mak b/tests/fate/libavformat.mak
index 09649fed11..4125dde0b4 100644
--- a/tests/fate/libavformat.mak
+++ b/tests/fate/libavformat.mak
@@ -12,6 +12,10 @@ fate-rename: libavformat/tests/rename$(EXESUF)
 fate-rename: CMD = run libavformat/tests/rename$(EXESUF)
 fate-rename: CMP = null
 
+FATE_LIBAVFORMAT-$(CONFIG_HTTP_PROTOCOL) += fate-http-status-line
+fate-http-status-line: libavformat/tests/http$(EXESUF)
+fate-http-status-line: CMD = run libavformat/tests/http$(EXESUF)
+
 FATE_LIBAVFORMAT-$(CONFIG_NETWORK) += fate-noproxy
 fate-noproxy: libavformat/tests/noproxy$(EXESUF)
 fate-noproxy: CMD = run libavformat/tests/noproxy$(EXESUF)
diff --git a/tests/ref/fate/http-status-line b/tests/ref/fate/http-status-line
new file mode 100644
index 0000000000..20ce4b0889
--- /dev/null
+++ b/tests/ref/fate/http-status-line
@@ -0,0 +1,28 @@
+"HTTP/1.1 200 OK"
+  version="1.1" code=200 willclose=0 reason=" OK"
+"HTTP/1.0 200 OK"
+  version="1.0" code=200 willclose=1 reason=" OK"
+"HTTP/1.1 404 Not Found"
+  version="1.1" code=404 willclose=0 reason=" Not Found"
+"HTTP/1.1 500 Internal Server Error"
+  version="1.1" code=500 willclose=0 reason=" Internal Server Error"
+"HTTP/1.1 204 No Content"
+  version="1.1" code=204 willclose=0 reason=" No Content"
+"HTTP/1.1 200"
+  version="1.1" code=200 willclose=0 reason=""
+"HTTP/1.1"
+  version="1.1" code=0 willclose=0 reason=""
+"HTTP/1.1 OK"
+  version="1.1" code=0 willclose=0 reason="OK"
+"HTTP"
+  version="" code=0 willclose=0 reason=""
+"200 OK"
+  version="" code=0 willclose=0 reason=""
+"NOT A STATUS LINE"
+  version="" code=0 willclose=0 reason=""
+""
+  version="" code=0 willclose=0 reason=""
+"ICY 200 OK"
+  version="" code=0 willclose=0 reason=""
+"ICY 404 Not Found"
+  version="" code=0 willclose=0 reason=""
-- 
2.52.0


>From 023b69c97bc0ac2b709cfc804abed6d95b375b28 Mon Sep 17 00:00:00 2001
From: Romain Beauxis <[email protected]>
Date: Fri, 21 Aug 2026 08:27:25 -0500
Subject: [PATCH 3/3] avformat/http: reject malformed response status lines

A status line carrying no HTTP version parsed as status code 0, which
check_http_code() then accepted as a success: the body of an error reply, or of
a reply from something that is not an HTTP server at all, was handed to the
demuxer.

The request line parser in the same function already rejects a version string
that does not start with "HTTP/"; do the same for responses, and reject a
status line whose code is missing or not a number.
---
 libavformat/http.c              | 18 ++++++++++++------
 tests/ref/fate/http-status-line | 16 ++++++++--------
 2 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/libavformat/http.c b/libavformat/http.c
index 50fd88970d..26130a20bc 100644
--- a/libavformat/http.c
+++ b/libavformat/http.c
@@ -897,20 +897,26 @@ int ff_http_parse_status_line(void *logctx, const char *line, HTTPStatusLine *st
 
     memset(st, 0, sizeof(*st));
 
-    if (av_strncasecmp(p, "HTTP/1.0", 8) == 0)
+    if (av_strncasecmp(p, "HTTP/", 5)) {
+        av_log(logctx, AV_LOG_ERROR, "Malformed HTTP status line.\n");
+        return AVERROR_INVALIDDATA;
+    }
+    p += 5;
+
+    if (av_strncasecmp(line, "HTTP/1.0", 8) == 0)
         st->willclose = 1;
 
-    while (*p != '/' && *p != '\0')
-        p++;
-    while (*p == '/')
-        p++;
     av_strlcpy(st->version, p, sizeof(st->version));
 
     while (!av_isspace(*p) && *p != '\0')
         p++;
     while (av_isspace(*p))
         p++;
-    st->code   = strtol(p, &end, 10);
+    st->code = strtol(p, &end, 10);
+    if (end == p) {
+        av_log(logctx, AV_LOG_ERROR, "Malformed HTTP status code.\n");
+        return AVERROR_INVALIDDATA;
+    }
     st->reason = end;
 
     av_log(logctx, AV_LOG_TRACE, "http_code=%d\n", st->code);
diff --git a/tests/ref/fate/http-status-line b/tests/ref/fate/http-status-line
index 20ce4b0889..05ce524573 100644
--- a/tests/ref/fate/http-status-line
+++ b/tests/ref/fate/http-status-line
@@ -11,18 +11,18 @@
 "HTTP/1.1 200"
   version="1.1" code=200 willclose=0 reason=""
 "HTTP/1.1"
-  version="1.1" code=0 willclose=0 reason=""
+  rejected
 "HTTP/1.1 OK"
-  version="1.1" code=0 willclose=0 reason="OK"
+  rejected
 "HTTP"
-  version="" code=0 willclose=0 reason=""
+  rejected
 "200 OK"
-  version="" code=0 willclose=0 reason=""
+  rejected
 "NOT A STATUS LINE"
-  version="" code=0 willclose=0 reason=""
+  rejected
 ""
-  version="" code=0 willclose=0 reason=""
+  rejected
 "ICY 200 OK"
-  version="" code=0 willclose=0 reason=""
+  rejected
 "ICY 404 Not Found"
-  version="" code=0 willclose=0 reason=""
+  rejected
-- 
2.52.0

_______________________________________________
ffmpeg-devel mailing list -- [email protected]
To unsubscribe send an email to [email protected]