Skip to content

JS: torrent content as a remote flow source#2324

Merged
semmle-qlci merged 2 commits into
github:masterfrom
esbena:js/torrent-as-remote-source
Nov 14, 2019
Merged

JS: torrent content as a remote flow source#2324
semmle-qlci merged 2 commits into
github:masterfrom
esbena:js/torrent-as-remote-source

Conversation

@esbena
Copy link
Copy Markdown
Contributor

@esbena esbena commented Nov 14, 2019

This PR makes us treat most content of torrents parsed by parse-torrent as untrusted. Such torrent content was the taint source in CVE-2019-15782 (we do not flag this vulnerability yet, we are missing a flow step elsewhere).

Implementationwise, I would have liked parseTorrent(...) to be a labelled RemoteFlowSource, and then let the flow label mechanism ensure that we only propagate through certain properties of the parsed torrent, but that requires unreasonable adjustments in all Configurations.

Instead, I have used type tracking to track the parsed torrent, and then used certain property reads of that tracked torrent as ordinary RemoteFlowSources. This leads to sub-optimal sources in the path-problem alerts, but it is the same pattern as the one we use for RequestInputAccess: we do not provide a link to the request object itself, but rather a use of it.

Generally speaking, this new source is related to the js/zipslip sources where we model that something is expected to be provided by external parties and thus should not be trusted. We should investigate such sources more thoroughly, and perhaps even deprecate js/zipslip in favor of a js/path-injection with zip-file content as a RemoteFlowSource.

Evaluation

  • 3 likely TPs among some packages that have parse-torrent as a dependency.
  • sanity check on nightly.slugs (ongoing)

@esbena esbena added the JS label Nov 14, 2019
@esbena esbena requested a review from a team as a code owner November 14, 2019 13:04
Copy link
Copy Markdown
Contributor

@max-schaefer max-schaefer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, assuming the evaluation works out (but I don't see why it wouldn't).

@esbena
Copy link
Copy Markdown
Contributor Author

esbena commented Nov 14, 2019

Uninteresting evaluation. Good to go.

@semmle-qlci semmle-qlci merged commit 0638907 into github:master Nov 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants