Skip to content

Output is not encoded: three reproducible XSS vectors, plus the build is broken on Node 22+ #57

Description

@justin-peacock

First, thanks for IVFi. It is a genuinely nice piece of software and it is the reason I went looking at the code at all.

While going through master I found that output is never encoded, and I was able to reproduce three cross-site scripting vectors against a stock build. I have fixed them in a fork and I am happy to open PRs here for any or all of it, or for you to just take the patches. I tried to report this privately first, but private vulnerability reporting is not enabled on this repository, so there was no non-public channel available.

The root cause

There is no call to htmlspecialchars() or htmlentities() anywhere in src/php/template.php. Helpers::createElement() concatenates attribute values and inner text straight into a string, and nothing upstream of it encodes either.

What I could reproduce

Each of these was reproduced end to end against a build from master, served with PHP's built-in server.

1. X-Indexer-Prepend-Path request header. The header is read from the request unconditionally and prepended, unencoded, to every href on the page. This one needs no file, no directory and no prior access, just a header on an ordinary GET:

curl -H 'X-Indexer-Prepend-Path: x" onerror="alert(7)" data-z="' http://host/
<a href="/x" onerror="alert(7)" data-z="">
<a href="/x" onerror="alert(7)" data-z="/dir">

It is documented as something a reverse proxy sets, but nothing enforces that it came from one. If a cache sits in front of the indexer and does not key on this header, one poisoned request would serve the payload to everyone after it.

2. Filename. A file named:

pwn" onmouseover="alert(1) <img src=x onerror=alert(document.domain)>.jpg

breaks out of both the attribute and the text node of its own row. Any directory where people can upload or otherwise influence filenames becomes a persistent store.

3. Directory name. The same payload in a directory name lands in the breadcrumb from makePathClickable() and in the page <title> via the format.title sprintf, producing a live <script> element.

Smaller things found alongside

  • The top-level catch echoes $e directly, which stringifies to include the stack trace and absolute filesystem paths, and it is not gated on debug. Requesting a path that does not exist is enough to read it.
  • Digest auth compares the response with !=, which is neither constant time nor type safe. Nonces come from uniqid() and are never checked when the client returns them, so there is no replay protection and nc is ignored.
  • isAboveCurrent() decides containment by string prefix, so a base of /srv/pub also matches the sibling /srv/pub-secret. Reaching it needs a server that forwards unnormalized traversal, so it is defence in depth rather than a live hole in most setups.
  • The config carried in the built indexer.php has 'debug' => true, though the internal $defaults uses false, so an unedited deployment logs to the console and defeats asset caching.
  • A path that does not exist answers 500. Since the page links a favicon, any deployment without one hits that on every single view.

Also: the build is broken on Node 22+

Unrelated to the above, but probably why the project looks stalled from outside: webpack.config.js uses assert { type: 'json' }, removed in Node 22, so the config will not parse and a fresh clone cannot be built on any current LTS. CI does not catch it because it pins Node 20.

You already fixed this on the deps branch in February 2025 (91d9fdd) and it just never landed on master. That one commit would unblock anyone trying to build today.

I also found that npm run lint covers almost nothing: npm run uses sh, where ** matches a single directory level, so ./src/core/**/*.ts reaches 20 of 42 files and ./src/css/**/*.scss matches none at all, meaning stylelint has never linted anything. Quoting the globs so the tools expand them fixes it.

What I have

Fixes for all of the above, plus a PHPUnit suite that covers them, are here:

justin-peacock#1

The approach on the encoding was to add one escaping primitive and make createElement() encode attribute values always and inner text by default, with an explicit opt-out for the call sites that pass markup it assembled itself. script and style are handled separately since encoding their contents would corrupt the CSS and JS. On a benign tree the rendered page is byte identical to the previous build apart from two intended changes.

Built against the pre-fix indexer, 19 of the 40 tests in that suite fail, and they map onto the findings above.

Happy to split this into separate, smaller PRs against master if that is easier to review, or to leave it if you would rather take a different approach. Either way I wanted to make sure this was reported rather than sitting only in a fork.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions