Visitar URL original
Store YAML nested file references are not contained to the store file's directory · Issue #736 · openfga/cli · GitHub
Skip to content

Store YAML nested file references are not contained to the store file's directory #736

Closed

Description

@SoulPancake

Summary

Nested file references in store YAML are resolved with path.Join(basePath, value) and no containment check, so a ../ prefix escapes the directory holding the store file. fga store import and fga model test then read that file.

Reported by @bugbunny-research.

Affected code

Four fields in internal/storetest/storedata.go, all resolved the same way:

field line
model_file 105
tuple_file 205
tuple_files[] 223
per-test tuple_file 247

Each joined path goes straight to authorizationmodel.ReadFromFile (internal/authorizationmodel/read-from-input.go:37) or tuplefile.ReadTupleFile (internal/tuplefile/read.go:19), both calling os.ReadFile.

path.Join runs path.Clean, which collapses .., so path.Join("/tmp/base", "../outside/secret.txt") yields /tmp/outside/secret.txt. Nothing checks the result is still under basePath. StoreData.Validate() (storedata.go:166) only checks user/object conflicts in test assertions. The !filepath.IsAbs(fileName) check at read-from-input.go:38 guards the top-level store file path, not nested references.

Note absolute paths are already harmless: path.Join("/base", "/etc/passwd") returns /base/etc/passwd. Only .. escapes.

Reproduction

mkdir -p /tmp/traversal-demo/sub
echo "contents outside the store dir" > /tmp/traversal-demo/secret.txt

cat > /tmp/traversal-demo/sub/store.fga.yaml <<'YAML'
name: traversal-demo
model_file: ../secret.txt
tuples: []
tests: []
YAML

fga store import --api-url http://localhost:8080 \
  --file /tmp/traversal-demo/sub/store.fga.yaml

Reads secret.txt from one directory up and submits it as an authorization model; the API rejects it, but the file was read. Same via tuple_file, tuple_files, and a per-test tuple_file.

Impact

  • Reads a file outside the base directory, limited to files the invoking user can already read. Content doesn't leave the process except as a rejected API payload.
  • /dev/zero as a file field makes os.ReadFile grow an unbounded buffer until OOM. A FIFO with no writer blocks instead.
  • No integrity impact. Writing supplied models and tuples is what store import does by design. For a traversed file to be written as tuples it needs a .json/.yaml/.yml/.jsonl/.csv extension (read.go:26 switches on path.Ext; anything else errors) and contents that parse as valid tuples.

Fix

func safeJoin(basePath, relPath string) (string, error) {
	joined := filepath.Join(basePath, relPath)

	rel, err := filepath.Rel(basePath, joined)
	if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) {
		return "", fmt.Errorf("file reference %q escapes the store file's directory", relPath)
	}

	return joined, nil
}

filepath.Rel is lexical, so this does not cover symlinks — that would need filepath.EvalSymlinks and would break intentionally symlinked fixtures, so it's a separate call.

Tasks

  • safeJoin helper in internal/storetest
  • Use it at all four sites
  • Tests: traversal rejected at each site, relative paths and subdirectories still work
  • Decide separately on a read size limit (/dev/zero) and symlink containment

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type

Projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions