From 492cefdeb29c50d2298382260f31e3c0be26ccfa Mon Sep 17 00:00:00 2001 From: Joe Freeman Date: Sat, 16 May 2026 09:05:34 +0100 Subject: [PATCH 1/2] Fix blob store path traversal issue --- server/lib/coflux/handlers/blobs.ex | 59 ++++++++++++++++++----------- 1 file changed, 37 insertions(+), 22 deletions(-) diff --git a/server/lib/coflux/handlers/blobs.ex b/server/lib/coflux/handlers/blobs.ex index 23213ef6..5092ab4d 100644 --- a/server/lib/coflux/handlers/blobs.ex +++ b/server/lib/coflux/handlers/blobs.ex @@ -3,6 +3,8 @@ defmodule Coflux.Handlers.Blobs do alias Coflux.{Auth, Utils} + @key_regex ~r/\A[0-9a-f]{64}\z/ + def init(req, opts) do bindings = :cowboy_req.bindings(req) req = set_cors_headers(req) @@ -39,40 +41,50 @@ defmodule Coflux.Handlers.Blobs do end defp handle(req, "HEAD", key, opts) do - exists = File.exists?(blob_path(key)) - status = if exists, do: 200, else: 404 + status = + cond do + not valid_key?(key) -> 404 + File.exists?(blob_path(key)) -> 200 + true -> 404 + end + req = :cowboy_req.reply(status, %{}, req) {:ok, req, opts} end defp handle(req, "GET", key, opts) do - case File.read(blob_path(key)) do - {:ok, content} -> - req = :cowboy_req.reply(200, %{}, content, req) - {:ok, req, opts} - - {:error, :enoent} -> + with true <- valid_key?(key), + {:ok, content} <- File.read(blob_path(key)) do + req = :cowboy_req.reply(200, %{}, content, req) + {:ok, req, opts} + else + _ -> req = :cowboy_req.reply(404, %{}, "Not found", req) {:ok, req, opts} end end defp handle(req, "PUT", key, opts) do - {:ok, temp_path} = Briefly.create() - - case File.open!(temp_path, [:write], &read_body(req, &1)) do - {:ok, req, hash} -> - req = - if key == Base.encode16(hash, case: :lower) do - path = blob_path(key) - path |> Path.dirname() |> File.mkdir_p!() - :ok = move_file(temp_path, path) - :cowboy_req.reply(204, req) - else - json_error_response(req, "hash_mismatch") - end + if valid_key?(key) do + {:ok, temp_path} = Briefly.create() + + case File.open!(temp_path, [:write], &read_body(req, &1)) do + {:ok, req, hash} -> + req = + if key == Base.encode16(hash, case: :lower) do + path = blob_path(key) + path |> Path.dirname() |> File.mkdir_p!() + :ok = move_file(temp_path, path) + :cowboy_req.reply(204, req) + else + json_error_response(req, "hash_mismatch") + end - {:ok, req, opts} + {:ok, req, opts} + end + else + req = json_error_response(req, "invalid_key") + {:ok, req, opts} end end @@ -80,6 +92,9 @@ defmodule Coflux.Handlers.Blobs do Utils.data_path("blobs/#{a}/#{b}/#{c}") end + defp valid_key?(key) when is_binary(key), do: Regex.match?(@key_regex, key) + defp valid_key?(_), do: false + defp read_body(req, file, hash \\ nil) do hash = hash || :crypto.hash_init(:sha256) From c0a458046778fcc227aaaa8591c97697934137d1 Mon Sep 17 00:00:00 2001 From: Joe Freeman Date: Sat, 16 May 2026 09:09:58 +0100 Subject: [PATCH 2/2] Add blobs tests --- tests/test_blobs.py | 118 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 118 insertions(+) create mode 100644 tests/test_blobs.py diff --git a/tests/test_blobs.py b/tests/test_blobs.py new file mode 100644 index 00000000..4d639be3 --- /dev/null +++ b/tests/test_blobs.py @@ -0,0 +1,118 @@ +"""Tests for the /blobs/:key HTTP endpoint.""" + +import hashlib +import http.client +import json +import os + +import pytest + + +def _conn(server, project_id): + """A raw HTTP connection with the project Host header.""" + c = http.client.HTTPConnection("127.0.0.1", server.port, timeout=5) + return c, {"Host": f"{project_id}.localhost:{server.port}"} + + +def _request(server, project_id, method, raw_path, body=None): + """Send a request with the path passed verbatim (no normalisation).""" + conn, headers = _conn(server, project_id) + try: + conn.request(method, raw_path, body=body, headers=headers) + resp = conn.getresponse() + data = resp.read() + return resp.status, data + finally: + conn.close() + + +def _hex_sha256(b): + return hashlib.sha256(b).hexdigest() + + +# Keys that must be rejected without touching the filesystem. One +# representative per rejection class: a traversal exploit, +# a wrong-length key, and a right-length non-hex key. +INVALID_KEYS = [ + "....%2F..%2F..%2F..%2Fetc%2Fpasswd", + "abc", + "z" * 64, +] + + +def test_put_then_get_roundtrip(server, project_id): + """A blob written with its real SHA-256 key can be read back.""" + body = b"hello, blobs" + key = _hex_sha256(body) + + status, _ = _request(server, project_id, "PUT", f"/blobs/{key}", body=body) + assert status == 204 + + status, data = _request(server, project_id, "GET", f"/blobs/{key}") + assert status == 200 + assert data == body + + status, _ = _request(server, project_id, "HEAD", f"/blobs/{key}") + assert status == 200 + + +def test_put_rejects_hash_mismatch(server, project_id): + """A valid-format key that doesn't match the body content is rejected.""" + # Format-valid key, but not the hash of the body. + bogus_key = "0" * 64 + status, data = _request( + server, project_id, "PUT", f"/blobs/{bogus_key}", body=b"some body" + ) + assert status == 400 + assert json.loads(data)["error"] == "hash_mismatch" + + +@pytest.mark.parametrize("key", INVALID_KEYS) +def test_get_rejects_invalid_key(server, project_id, key): + status, _ = _request(server, project_id, "GET", f"/blobs/{key}") + assert status == 404 + + +@pytest.mark.parametrize("key", INVALID_KEYS) +def test_head_rejects_invalid_key(server, project_id, key): + status, _ = _request(server, project_id, "HEAD", f"/blobs/{key}") + assert status == 404 + + +@pytest.mark.parametrize("key", INVALID_KEYS) +def test_put_rejects_invalid_key(server, project_id, key): + status, data = _request( + server, project_id, "PUT", f"/blobs/{key}", body=b"anything" + ) + assert status == 400 + assert json.loads(data)["error"] == "invalid_key" + + +def test_no_files_created_outside_blobs_dir(isolated_server): + """Malicious keys must not create any files or dirs on disk. + + Uses an isolated server so we can inspect the data dir in isolation + and assert that only the expected ``blobs///`` layout + (or nothing at all) appears. + """ + srv, host, pid = isolated_server + project_id = host.split(".", 1)[0] + + # Hit every invalid-key vector across all three verbs. + for method in ("GET", "HEAD", "PUT"): + for key in INVALID_KEYS: + _request(srv, project_id, method, f"/blobs/{key}", body=b"x") + + data_dir = srv.data_dir + # No traversal should have created files outside ``data_dir``. + assert os.path.isdir(data_dir), "server data dir disappeared" + + # If a ``blobs`` dir exists at all, it must contain only well-formed + # two-char subdirs (none of the invalid keys above start with two hex + # chars and a slash, so the dir should not exist). + blobs_dir = os.path.join(data_dir, "blobs") + if os.path.exists(blobs_dir): + for entry in os.listdir(blobs_dir): + assert len(entry) == 2 and all( + c in "0123456789abcdef" for c in entry + ), f"unexpected blob shard: {entry!r}"