diff --git a/changes-entries/mime-magic-negative-offset.txt b/changes-entries/mime-magic-negative-offset.txt
new file mode 100644
index 00000000000..c8566b8f87a
--- /dev/null
+++ b/changes-entries/mime-magic-negative-offset.txt
@@ -0,0 +1,3 @@
+ *) mod_mime_magic: Reject negative indirect offsets in mget() so a crafted
+ file cannot drive an out-of-bounds read of the sniff buffer.
+ [arshiya tabasum]
diff --git a/modules/metadata/mod_mime_magic.c b/modules/metadata/mod_mime_magic.c
index c15d11b7ff7..2a5150f9083 100644
--- a/modules/metadata/mod_mime_magic.c
+++ b/modules/metadata/mod_mime_magic.c
@@ -1085,7 +1085,7 @@ static unsigned long signextend(server_rec *s, struct magic *m, unsigned long v)
case LONG:
case BELONG:
case LELONG:
- v = (long) v;
+ v = (apr_int32_t) v;
break;
case STRING:
break;
@@ -1802,16 +1802,16 @@ static int mconvert(request_rec *r, union VALUETYPE *p, struct magic *m)
return 1;
case BELONG:
case BEDATE:
- p->l = (long)
- ((p->hl[0] << 24) | (p->hl[1] << 16) | (p->hl[2] << 8) | (p->hl[3]));
+ p->l = ((apr_uint32_t) p->hl[0] << 24) | ((apr_uint32_t) p->hl[1] << 16)
+ | (p->hl[2] << 8) | (p->hl[3]);
return 1;
case LESHORT:
p->h = (short) ((p->hs[1] << 8) | (p->hs[0]));
return 1;
case LELONG:
case LEDATE:
- p->l = (long)
- ((p->hl[3] << 24) | (p->hl[2] << 16) | (p->hl[1] << 8) | (p->hl[0]));
+ p->l = ((apr_uint32_t) p->hl[3] << 24) | ((apr_uint32_t) p->hl[2] << 16)
+ | (p->hl[1] << 8) | (p->hl[0]);
return 1;
default:
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(01538)
@@ -1826,7 +1826,7 @@ static int mget(request_rec *r, union VALUETYPE *p, unsigned char *s,
{
long offset = m->offset;
- if (offset + sizeof(union VALUETYPE) > nbytes)
+ if (offset < 0 || (apr_size_t)offset + sizeof(union VALUETYPE) > nbytes)
return 0;
memcpy(p, s + offset, sizeof(union VALUETYPE));
@@ -1848,7 +1848,7 @@ static int mget(request_rec *r, union VALUETYPE *p, unsigned char *s,
break;
}
- if (offset + sizeof(union VALUETYPE) > nbytes)
+ if (offset < 0 || (apr_size_t)offset + sizeof(union VALUETYPE) > nbytes)
return 0;
memcpy(p, s + offset, sizeof(union VALUETYPE));
diff --git a/test/modules/metadata/env.py b/test/modules/metadata/env.py
index 038d478968a..608b9e095d5 100644
--- a/test/modules/metadata/env.py
+++ b/test/modules/metadata/env.py
@@ -13,13 +13,28 @@ def __init__(self, env: 'HttpdTestEnv'):
super().__init__(env=env)
self.add_source_dir(os.path.dirname(inspect.getfile(MetadataTestSetup)))
self.add_modules(["mime", "mime_magic"])
+ # mod_mime_libmagic needs libmagic at build time, so it must not
+ # be a hard requirement; its tests skip when it is absent.
+ self.add_optional_modules(["mime_libmagic"])
class MetadataTestEnv(HttpdTestEnv):
def __init__(self, pytestconfig=None):
super().__init__(pytestconfig=pytestconfig)
- self.add_httpd_log_modules(["mime_magic", "core"])
+ # Only raise the log level for mod_mime_libmagic when it is built:
+ # a LogLevel for an unloaded module is a fatal config error, which
+ # would break the whole suite (including the mod_mime_magic tests)
+ # on platforms without libmagic, such as Windows.
+ log_modules = ["mime_magic", "core"]
+ if self.has_libmagic_module:
+ log_modules.insert(1, "mime_libmagic")
+ self.add_httpd_log_modules(log_modules)
def setup_httpd(self, setup: HttpdTestSetup = None):
super().setup_httpd(setup=MetadataTestSetup(env=self))
+
+ @property
+ def has_libmagic_module(self) -> bool:
+ """Whether mod_mime_libmagic was built."""
+ return self.has_shared_module("mime_libmagic")
diff --git a/test/modules/metadata/samples.py b/test/modules/metadata/samples.py
new file mode 100644
index 00000000000..310cee310f0
--- /dev/null
+++ b/test/modules/metadata/samples.py
@@ -0,0 +1,49 @@
+# Sample file contents for content type detection tests. Each is
+# served without an extension, so mod_mime sets no type and the magic
+# modules have to derive one from the content. mod_mime_magic only
+# applies its magic rules to files of at least 64 bytes, so samples
+# meant to match a rule are padded past that.
+import gzip
+import os
+
+SAMPLES = {
+ "html": b"\nhello\n\n",
+ "html-doctype": b"\nt"
+ b"hi\n",
+ # (no commas: libmagic 5.45 calls lines with a consistent number
+ # of commas text/csv)
+ "text": b"hello world this is plain ascii text\n"
+ b"with a second line of a different length\n" * 2,
+ "text-utf8": "café naïve résumé\n".encode() * 4,
+ "text-latin1": b"caf\xe9 na\xefve\n" * 4,
+ "csrc": b"#include \n\nint main(int argc, char **argv)\n{\n"
+ b" printf(\"hi\\n\");\n return 0;\n}\n",
+ "json": b'{"a": 1, "b": [1, 2, 3], "c": {"d": "e", "f": "some more text"}}\n',
+ "xml": b"\n1234\n",
+ "rfc822": b"From: a@example.com\nTo: b@example.com\nSubject: hi there\n\n"
+ b"the body of the message\n",
+ "shell": b"#!/bin/sh\necho hi\necho there\necho this is a shell script\n"
+ b"exit 0\n",
+ "png": b"\x89PNG\r\n\x1a\n\x00\x00\x00\rIHDR"
+ b"\x00\x00\x00\x10\x00\x00\x00\x10\x08\x06\x00\x00\x00\x1f\xf3\xffa"
+ + b"\x00" * 64,
+ "gif": b"GIF89a\x10\x00\x10\x00\x80\x00\x00" + b"\x00" * 64,
+ "pdf": b"%PDF-1.4\n%\xe2\xe3\xcf\xd3\n1 0 obj\n<< /Type /Catalog >>\nendobj\n",
+ "gzip": gzip.compress(bytes(range(32, 127)) * 4, mtime=0),
+ "elf": b"\x7fELF\x02\x01\x01\x00" + b"\x00" * 8
+ + b"\x02\x00\x3e\x00\x01\x00\x00\x00" + b"\x00" * 100,
+ # deterministic bytes which libmagic reports as application/octet-stream
+ "binary": bytes((i * 7919) % 256 for i in range(4096)),
+ "empty": b"",
+}
+
+
+def write_samples(doc_dir):
+ """Write every sample into doc_dir, plus png.txt (PNG content
+ with a .txt extension)."""
+ os.makedirs(doc_dir, exist_ok=True)
+ for name, content in SAMPLES.items():
+ with open(os.path.join(doc_dir, name), "wb") as f:
+ f.write(content)
+ with open(os.path.join(doc_dir, "png.txt"), "wb") as f:
+ f.write(SAMPLES["png"])
diff --git a/test/modules/metadata/test_001_mime_magic.py b/test/modules/metadata/test_001_mime_magic.py
index 29b3acc59f3..d5cf54efe56 100644
--- a/test/modules/metadata/test_001_mime_magic.py
+++ b/test/modules/metadata/test_001_mime_magic.py
@@ -1,5 +1,7 @@
import os
import re
+import sys
+
import pytest
from pyhttpd.conf import HttpdConf
@@ -17,6 +19,14 @@ def _class_scope(self, env):
with open(magic_file, "w") as f:
f.write("0\tstring\tTESTMAGIC\tapplication/x-test-magic\tx-test-encoding\n")
f.write("0\tstring\tTESTNOTE\tapplication/x-test-note (some note)\n")
+ # 32-bit values with the high bit set, signed and unsigned
+ f.write("0\tbelong\t0xcafebabe\tapplication/x-test-belong\n")
+ f.write("0\tulelong\t0xcafebabe\tapplication/x-test-ulelong\n")
+ # an indirect rule whose computed offset is (long at 0) - 1;
+ # for a file beginning with a zero long that is -1, which a
+ # bounds check testing only the upper bound would let through
+ f.write("0\tlelong\t0\tx\n")
+ f.write(">(0.l-1)\tbyte\tx\tapplication/x-test-indir\n")
# Files without an extension, so mod_mime sets no type and
# mod_mime_magic has to derive one from the content.
@@ -29,6 +39,13 @@ def _class_scope(self, env):
f.write(b"TESTNOTE" + b" and some more content" * 4 + b"\n")
with open(os.path.join(doc_dir, "html-plain"), "wb") as f:
f.write(b"\nhello\n")
+ with open(os.path.join(doc_dir, "belong"), "wb") as f:
+ f.write(b"\xca\xfe\xba\xbe" + b"\0" * 64)
+ with open(os.path.join(doc_dir, "ulelong"), "wb") as f:
+ f.write(b"\xbe\xba\xfe\xca" + b"\0" * 64)
+ # zero long at offset 0, then padding past the 64-byte minimum
+ with open(os.path.join(doc_dir, "indir"), "wb") as f:
+ f.write(b"\0" * 100)
# HTML token followed by an ESC byte.
with open(os.path.join(doc_dir, "html-escape"), "wb") as f:
f.write(b"\n\x1b[1mhello\x1b[0m\n")
@@ -81,3 +98,31 @@ def test_metadata_001_04_softmagic_note(self, env):
assert env.httpd_error_log.scan_recent(
re.compile(r'.*AH10623: .*ignoring invalid content encoding.*'))
env.httpd_error_log.ignore_recent(lognos=["AH10623"])
+
+ # a long value with the high bit set matches the rule's value,
+ # whether compared signed or unsigned
+ # The 0xcafebabe rule value only fits a 64-bit C long: on an LLP64
+ # platform (Windows) long is 32 bits, where the parser's strtol()
+ # cannot represent it and the sign-extension corner this guards
+ # against does not arise.
+ @pytest.mark.skipif(sys.platform == "win32",
+ reason="magic value needs a 64-bit long")
+ @pytest.mark.parametrize("name", ["belong", "ulelong"])
+ def test_metadata_001_05_long_high_bit(self, env, name):
+ url = env.mkurl("http", "test1", f"/magic/{name}")
+ r = env.curl_get(url)
+ assert r.response, "no response: server may have crashed"
+ assert r.response["status"] == 200
+ assert r.response["header"]["content-type"] == f"application/x-test-{name}"
+
+ # an indirect rule computing a negative offset must not read outside
+ # the sniff buffer. The file begins with a zero long, so ">(0.l-1)"
+ # resolves to offset -1; without the fix that slips a bounds check
+ # testing only the upper bound and reads before the buffer, which a
+ # sanitizer build (the ASan and UBSan CI jobs) aborts on, leaving the
+ # request with no response.
+ def test_metadata_001_06_indirect_negative_offset(self, env):
+ url = env.mkurl("http", "test1", "/magic/indir")
+ r = env.curl_get(url)
+ assert r.response, "no response: server may have crashed"
+ assert r.response["status"] == 200
diff --git a/test/modules/metadata/test_002_mime_libmagic.py b/test/modules/metadata/test_002_mime_libmagic.py
new file mode 100644
index 00000000000..8ba335fb7cf
--- /dev/null
+++ b/test/modules/metadata/test_002_mime_libmagic.py
@@ -0,0 +1,89 @@
+import os
+import pytest
+
+from pyhttpd.conf import HttpdConf
+from .samples import write_samples
+
+
+def get_type(env, path):
+ """The Content-Type header of GET path on test1, or None."""
+ r = env.curl_get(env.mkurl("http", "test1", path))
+ assert r.response, "no response: server may have crashed"
+ assert r.response["status"] == 200
+ assert "content-encoding" not in r.response["header"]
+ return r.response["header"].get("content-type")
+
+
+class TestMimeLibmagic:
+
+ @pytest.fixture(autouse=True, scope='class')
+ def _class_scope(self, env):
+ if not env.has_libmagic_module:
+ pytest.skip("mod_mime_libmagic is not built")
+ write_samples(os.path.join(env.server_dir, "htdocs", "test1", "libmagic"))
+ conf = HttpdConf(env, extras={
+ 'base': "MimeLibmagic On",
+ })
+ conf.add_vhost_test1()
+ conf.install()
+ assert env.apache_restart() == 0
+
+ @pytest.mark.parametrize(["name", "ctype"], [
+ ("html", "text/html"),
+ ("html-doctype", "text/html"),
+ ("text", "text/plain"),
+ ("text-utf8", "text/plain"),
+ ("text-latin1", "text/plain"),
+ ("csrc", "text/x-c"),
+ ("json", "application/json"),
+ ("xml", "text/xml"),
+ ("rfc822", "message/rfc822"),
+ ("shell", "text/x-shellscript"),
+ ("png", "image/png"),
+ ("gif", "image/gif"),
+ ("pdf", "application/pdf"),
+ # no decompression support, the compressed file itself is typed
+ ("gzip", "application/gzip"),
+ # libmagic appends ", no program header" to the type
+ ("elf", "application/x-executable"),
+ # application/octet-stream from libmagic leaves the type unset
+ ("binary", None),
+ ("empty", "text/plain"),
+ ])
+ def test_metadata_002_01_types(self, env, name, ctype):
+ assert get_type(env, f"/libmagic/{name}") == ctype
+
+ # mod_mime's extension mapping wins over the content
+ def test_metadata_002_02_extension(self, env):
+ assert get_type(env, "/libmagic/png.txt") == "text/plain"
+
+
+class TestMimeLibmagicCharset:
+
+ @pytest.fixture(autouse=True, scope='class')
+ def _class_scope(self, env):
+ if not env.has_libmagic_module:
+ pytest.skip("mod_mime_libmagic is not built")
+ write_samples(os.path.join(env.server_dir, "htdocs", "test1", "libmagic"))
+ conf = HttpdConf(env, extras={
+ 'base': """
+ MimeLibmagic On
+ MimeLibmagicCharset On
+ """,
+ })
+ conf.add_vhost_test1()
+ conf.install()
+ assert env.apache_restart() == 0
+
+ @pytest.mark.parametrize(["name", "ctype"], [
+ ("html", "text/html; charset=us-ascii"),
+ ("text", "text/plain; charset=us-ascii"),
+ ("text-utf8", "text/plain; charset=utf-8"),
+ ("text-latin1", "text/plain; charset=iso-8859-1"),
+ # only text types get a charset
+ ("json", "application/json"),
+ ("png", "image/png"),
+ ("binary", None),
+ ])
+ def test_metadata_002_10_charset(self, env, name, ctype):
+ assert get_type(env, f"/libmagic/{name}") == ctype
diff --git a/test/modules/metadata/test_003_compare.py b/test/modules/metadata/test_003_compare.py
new file mode 100644
index 00000000000..cdc48c44f9b
--- /dev/null
+++ b/test/modules/metadata/test_003_compare.py
@@ -0,0 +1,85 @@
+import logging
+import os
+import pytest
+
+from pyhttpd.conf import HttpdConf
+from .samples import SAMPLES, write_samples
+
+log = logging.getLogger(__name__)
+
+# (sample, mod_mime_magic type, mod_mime_magic encoding, mod_mime_libmagic type)
+# mod_mime_magic uses the stock conf/magic and its token-based text
+# detection, which knows nothing of most of these, so many are untyped.
+EXPECTED = [
+ ("html", "text/html", None, "text/html"),
+ ("html-doctype", None, None, "text/html"),
+ ("text", None, None, "text/plain"),
+ ("text-utf8", None, None, "text/plain"),
+ ("text-latin1", None, None, "text/plain"),
+ ("csrc", "text/plain", None, "text/x-c"),
+ ("json", None, None, "application/json"),
+ ("xml", "text/xml", None, "text/xml"),
+ ("rfc822", "message/rfc822", "7bit", "message/rfc822"),
+ ("shell", None, None, "text/x-shellscript"),
+ ("png", "image/png", None, "image/png"),
+ ("gif", "image/gif", None, "image/gif"),
+ ("pdf", None, None, "application/pdf"),
+ ("gzip", "application/octet-stream", "x-gzip", "application/gzip"),
+ ("elf", None, None, "application/x-executable"),
+ ("binary", None, None, None),
+ ("empty", "text/plain", None, "text/plain"),
+ ("png.txt", "text/plain", None, "text/plain"),
+]
+
+
+def get_headers(env, host, path):
+ r = env.curl_get(env.mkurl("http", host, path))
+ assert r.response, "no response: server may have crashed"
+ assert r.response["status"] == 200, r.response
+ h = r.response["header"]
+ return h.get("content-type"), h.get("content-encoding")
+
+
+class TestCompare:
+ """The same files served by mod_mime_magic (vhost test1) and by
+ mod_mime_libmagic (vhost test2)."""
+
+ @pytest.fixture(autouse=True, scope='class')
+ def _class_scope(self, env):
+ if not env.has_libmagic_module:
+ pytest.skip("mod_mime_libmagic is not built")
+ for vhost in ["test1", "test2"]:
+ write_samples(os.path.join(env.server_dir, "htdocs", vhost, "cmp"))
+ # No MimeMagicFile in the main server, so test2 inherits no
+ # magic rules and only mod_mime_libmagic runs there.
+ conf = HttpdConf(env, extras={
+ f"test1.{env.http_tld}": f'MimeMagicFile "{env.prefix}/conf/magic"',
+ f"test2.{env.http_tld}": "MimeLibmagic On",
+ })
+ conf.add_vhost_test1()
+ conf.add_vhost_test2()
+ conf.install()
+ assert env.apache_restart() == 0
+
+ @pytest.mark.parametrize(["name", "mm_type", "mm_enc", "lm_type"], EXPECTED)
+ def test_metadata_003_01_mime_magic(self, env, name, mm_type, mm_enc, lm_type):
+ assert get_headers(env, "test1", f"/cmp/{name}") == (mm_type, mm_enc)
+
+ @pytest.mark.parametrize(["name", "mm_type", "mm_enc", "lm_type"], EXPECTED)
+ def test_metadata_003_02_libmagic(self, env, name, mm_type, mm_enc, lm_type):
+ assert get_headers(env, "test2", f"/cmp/{name}") == (lm_type, None)
+
+ # Log a side-by-side table of what the two modules actually report.
+ def test_metadata_003_03_report(self, env):
+ rows = [("sample", "mod_mime_magic", "mod_mime_libmagic")]
+ for name in list(SAMPLES) + ["png.txt"]:
+ mm = get_headers(env, "test1", f"/cmp/{name}")
+ lm = get_headers(env, "test2", f"/cmp/{name}")
+ fmt = lambda t: f"{t[0]}" + (f" ({t[1]})" if t[1] else "")
+ rows.append((name, fmt(mm), fmt(lm)))
+ widths = [max(len(r[i]) for r in rows) for i in range(3)]
+ table = "\n".join(" ".join(c.ljust(widths[i]) for i, c in enumerate(r))
+ for r in rows)
+ log.info("content types reported:\n%s", table)
+ with open(os.path.join(env.gen_dir, "mime-compare.txt"), "w") as f:
+ f.write(table + "\n")