Skip to content

Security1 publisher2 min readPublished

ocaml-cohttp stops percent-encoded ..%2f from escaping the docroot

The fix decodes a request path exactly once before removing dot segments, so ..%2f..%2f no longer resolves outside the docroot. Servers that gate access on path prefixes still have to call the new Cohttp.Path.normalise themselves.

The Watch · Security desk

Illustration accompanying ocaml-cohttp stops percent-encoded ..%2f from escaping the docroot

What happened

  • Cohttp.Path.resolve_local_file no longer escapes the docroot when handed percent-encoded traversal sequences such as ..%2f..%2f, and the URI path is now percent-decoded exactly once before dot segments are removed.
  • The release adds Cohttp.Path.normalise, and the changelog tells servers that make access control decisions on path segments to apply it to Request.uri, which does not normalise absolute-form or percent-encoded targets.
  • The cohttp-mirage static file server now normalises and percent-decodes the path of every request before a mirage-kv lookup, so /my%20file.txt retrieves the key my file.txt.

Compiled by The WatchSomething wrong?How this is made

Why it matters

  • exposure Any client that could reach a cohttp server serving files through resolve_local_file could read outside the docroot by encoding the slashes in a traversal string. No credentials were involved.
  • decision Each OCaml service that authorises on path prefixes now owns a code change rather than a version bump, and someone has to go find every callback that splits a request path before checking it.
  • constraint The cohttp-mirage upgrade is not behaviour-preserving for static-file unikernels: a store whose keys contain literal percent escapes will miss lookups that used to succeed.
  • constraint With no CVE, advisory or version range attached, an operator cannot answer whether a deployment is affected from a vulnerability feed and has to read the dependency's changelog to tell.

The order of operations was the bug. Strip `.` and `..` segments first and percent-decode afterwards, and a target like `..%2f..%2f` passes the stripper untouched, then arrives at the filesystem as `../../`. The fix decodes the URI path exactly once, and removes the dot segments after that [2]. Decoding once also closes the doubly-encoded variant: `%252e%252e` comes out of a single decode as `%2e%2e`, which is not a dot segment [14].

The second problem is not in the file resolver. `Request.uri` does not normalise absolute-form or percent-encoded targets [4]. An application that splits the request path on `/` and checks whether the first segment is `admin` is therefore inspecting whatever the client sent, and upgrading the library does not change that. The changelog's own example calls `Cohttp.Path.normalise` on the URI before the split, and answers not found for `"admin" :: _` when the request is not authorised [10].

Normalisation is off by default. "Normalisation is not applied by default, as existing code may depend on the present semantics, which are safe when not combined with local file resolution", the changelog says [5]. The traversal fix lands when the dependency is bumped. The access-control fix lands only when a maintainer edits a callback [15].

The work is one pull request. #1145 is credited to @avsm and Sapphire Livingstone, with review by @mdales, @edwintorok and @patricoferris [9], and five entries in the changelog trace back to it [13]. Two of those are in cohttp-mirage. The static file server now normalises the path of every request, including directory requests, before it looks a key up in the mirage-kv store, and keys are percent-decoded, so `/my%20file.txt` retrieves `my file.txt` [7]. The `request_fn` callback now receives the request URI unchanged, where a request that fell back to an index page used to receive a URI rewritten to that page [8].

`resolve_local_file` picked up a second behaviour change in the same pull request: it collapses empty path segments and drops a trailing slash, so `/dir//sub/` resolves to `docroot/dir/sub` rather than `docroot/dir//sub/` [6].

One unrelated correctness fix ships alongside it. In cohttp-eio, a body chunk handed to the reader across several `single_read` calls used to copy from offset 0 on the second and later partial deliveries, resending its first bytes in place of the next ones and corrupting bodies read with a buffer smaller than the chunk, credited to @jeong-sik in #1149 [11].

What to watch

  • Whether a CVE identifier and an affected version range are published for the resolve_local_file behaviour, neither of which the changelog carries.
  • Whether downstream OCaml and MirageOS servers add Cohttp.Path.normalise to the callbacks that authorise on path segments.
  • Whether cohttp turns normalisation on by default in a later major release, and what it breaks when it does.
Loading claim ledger
Loading source directory links
Loading share composer
Loading topic controls
Loading related stories