Skip to content

fix(tfjs-node): prevent path traversal in NodeFileSystem.loadWeights#8709

Open
adilburaksen wants to merge 1 commit into
tensorflow:masterfrom
adilburaksen:fix/loadWeights-path-traversal
Open

fix(tfjs-node): prevent path traversal in NodeFileSystem.loadWeights#8709
adilburaksen wants to merge 1 commit into
tensorflow:masterfrom
adilburaksen:fix/loadWeights-path-traversal

Conversation

@adilburaksen
Copy link
Copy Markdown

Summary

NodeFileSystem.loadWeights() constructs weight file paths by calling
path.join(dirName, path) where path comes directly from the
weightsManifest[].paths array in a user-supplied model.json. Node.js
path.join normalises .. components without any boundary restriction, so a
crafted manifest entry like "../../../etc/passwd" resolves to that file on
the host filesystem, which is then passed to readFile.

Minimal reproducer (no tfjs-node install needed — uses the exact vulnerable logic):

const {join, dirname} = require('path');
const fs = require('fs').promises;

// Exact copy of the vulnerable lines from file_system.ts:192-193
async function simulate(weightsManifest, modelPath) {
  const dirName = dirname(modelPath);
  for (const group of weightsManifest) {
    for (const p of group.paths) {
      const weightFilePath = join(dirName, p);   // no traversal check
      const buf = await fs.readFile(weightFilePath);
      console.log('read:', buf.toString('utf8'));
    }
  }
}

With paths: ["../secret/credentials.txt"] and model at /app/model/model.json,
join resolves to /app/secret/credentials.txt and readFile reads it.
The raw bytes are then returned as the model's weightData and are
recoverable via model.getWeights()[i].data().

Fix

Resolve the model directory to its canonical path via realpath(), then verify
that each resolved weight path is a strict descendant before calling readFile.
The realpath() call also blocks symlink-based escapes where a symlink inside
the model directory points to a path outside it.

Test plan

  • loadLayersModel('file://...') with a normal model loads correctly
  • loadLayersModel('file://...') with weightsManifest.paths: ["../outside.bin"] throws Weight file path … is outside the model directory
  • loadLayersModel('file://...') with absolute path in weightsManifest.paths is rejected

Weight file paths in weightsManifest are joined with path.join(), which
normalises ".." components without any boundary check. A crafted model.json
can set paths like "../../../etc/passwd" to read arbitrary files that are
accessible to the Node.js process.

Fix: resolve the model directory to its canonical path via realpath(), then
verify that each resolved weight file path starts with that canonical base.
The realpath() call in the weight path catch handles the non-existent file
case while still blocking traversal via fake symlink chains.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant