Skip to content

fix(js): reject unsandboxed ccache writes in krbforge - #7482

Merged
Mzack9999 merged 1 commit into
devfrom
dwisiswant0/fix/js/reject-unsandboxed-ccache-writes-in-krbforge
Jun 23, 2026
Merged

fix(js): reject unsandboxed ccache writes in krbforge#7482
Mzack9999 merged 1 commit into
devfrom
dwisiswant0/fix/js/reject-unsandboxed-ccache-writes-in-krbforge

Conversation

@dwisiswant0

@dwisiswant0 dwisiswant0 commented Jun 20, 2026

Copy link
Copy Markdown
Member

Proposed changes

OutputFile was treated as a goimpacket option
instead of a fs sink. Since goimpacket writes the
ccache itself, krbforge had to enforce nuclei's
local file policy before building the ticket
config.

Add execution-aware JS wrappers, normalize
relative output paths into the template sandbox,
and deny outside paths unless local file access is
enabled. Keep empty output as in-memory only by
passing "-".

Proof

Checklist

  • Pull request is created against the dev branch
  • All checks passed (lint, unit/integration/regression tests etc.) with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

Summary by CodeRabbit

  • Bug Fixes

    • Restricted Kerberos ticket output paths to template directory when local file access is disabled; suggests using -allow-local-file-access flag for external paths.
    • Prevented default cache file creation in working directory.
  • Refactor

    • Refactored ticket forging functions for internal runtime integration.

`OutputFile` was treated as a goimpacket option
instead of a fs sink. Since goimpacket writes the
ccache itself, krbforge had to enforce nuclei's
local file policy before building the ticket
config.

Add execution-aware JS wrappers, normalize
relative output paths into the template sandbox,
and deny outside paths unless local file access is
enabled. Keep empty output as in-memory only by
passing "-".

Signed-off-by: Dwi Siswanto <git@dw1.io>
@coderabbitai

coderabbitai Bot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 60c7c097-e765-4339-99e6-046aa368bf96

📥 Commits

Reviewing files that changed from the base of the PR and between 1fe6025 and bb297c4.

📒 Files selected for processing (2)
  • pkg/js/libs/krbforge/krbforge.go
  • pkg/js/libs/krbforge/krbforge_test.go

Walkthrough

CreateGoldenTicket and CreateSilverTicket in krbforge.go are converted from direct Go APIs to goja/JS runtime wrappers. A new normalizeOutputFile function enforces nuclei sandbox path rules: paths outside the templates directory are rejected unless -allow-local-file-access is enabled. Tests cover path rejection, acceptance, normalization, and runtime panic behavior.

Changes

Kerberos Ticket Forging: goja wrappers and sandbox output path enforcement

Layer / File(s) Summary
goja wrapper conversion for CreateGoldenTicket and CreateSilverTicket
pkg/js/libs/krbforge/krbforge.go
Imports expanded for goja and nuclei runtime packages. CreateGoldenTicket and CreateSilverTicket converted from (TicketRequest) -> (*Ticket, error) Go APIs to (goja.FunctionCall, *goja.Runtime) -> goja.Value wrappers that extract executionID, export JS arguments via new helpers, route through internal helpers, and propagate errors via nj.ThrowError.
Sandbox-aware normalizeOutputFile and buildConfig
pkg/js/libs/krbforge/krbforge.go
buildConfig gains an executionID parameter and delegates to new normalizeOutputFile. That function branches on protocolstate.IsLfaAllowed: with LFA enabled, relative paths are resolved into a temp directory; without LFA, non-absolute paths are mapped under the nuclei templates directory, and any resolved path escaping that directory is rejected with a message suggesting -allow-local-file-access. exportTicketRequest and exportOutputFile helpers convert goja values to Go types.
Sandboxing and runtime behavior tests
pkg/js/libs/krbforge/krbforge_test.go
Tests cover buildConfig rejecting outside-sandbox paths with LFA disabled, accepting inside-sandbox paths, normalizing relative paths into the templates sandbox, and allowing outside paths when LFA is enabled. A goja runtime test asserts CreateGoldenTicket panics with -allow-local-file-access is not enabled. A createSilverTicket test confirms empty output produces OutputFile of "-" without writing a default ccache file. Helpers setTemplateDir, setLocalFileAccess, and validTicketRequest support test setup.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐇 Hippity-hop through the ticket forge,
Old plain Go calls got a goja gorge,
Paths outside sandbox? Rejected with flair,
-allow-local-file-access if you dare!
Tests now confirm every rule holds tight,
The rabbit stamps approved — all looks right! ✅

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix(js): reject unsandboxed ccache writes in krbforge' directly and specifically describes the main security fix implemented in the PR—preventing unauthorized credential cache file writes outside the sandbox.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch dwisiswant0/fix/js/reject-unsandboxed-ccache-writes-in-krbforge

Comment @coderabbitai help to get the list of available commands and usage tips.

@dwisiswant0

Copy link
Copy Markdown
Member Author

@coderabbitai create an issue for this, prefixed with [BUG].

@coderabbitai

coderabbitai Bot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

gh issue create \
  --repo "projectdiscovery/nuclei" \
  --title "[BUG] krbforge JS bindings do not enforce sandbox policy for ccache file writes" \
  --body "## Description

The JavaScript bindings for the \`krbforge\` module do not enforce Nuclei's local file access sandbox policy when writing credential cache (ccache) files via the \`OutputFile\` option.

## Root Cause

\`OutputFile\` is treated as a goimpacket library option rather than a filesystem sink. Since goimpacket handles the actual ccache file writing internally, \`krbforge\` was unable to enforce Nuclei's local file access policies before the ticket configuration was built. This means a malicious or misconfigured template could write ccache files to arbitrary filesystem locations without requiring \`-allow-local-file-access\` to be explicitly enabled by the user.

## Impact

- Templates using \`krbforge.CreateGoldenTicket\` or \`krbforge.CreateSilverTicket\` with an \`OutputFile\` pointing outside the sandbox directory could write ccache files to arbitrary paths on the host filesystem.
- The sandbox restriction (\`-allow-local-file-access\`) is bypassed entirely for this code path.

## Steps to Reproduce

Use a Nuclei template that calls \`krbforge.CreateGoldenTicket\` or \`krbforge.CreateSilverTicket\` with an \`OutputFile\` path pointing outside the template sandbox directory (e.g., \`/tmp/evil.ccache\`) without passing \`-allow-local-file-access\`. The file will be written without restriction.

## Expected Behavior

Paths outside the sandbox directory should be rejected unless \`-allow-local-file-access\` is explicitly enabled. Empty or unspecified output paths should default to in-memory operations (passing \`\"-\"\` to the underlying library) rather than writing to disk.

## Related

- Fixed in PR `#7482`

/cc `@dwisiswant0`"

Length of output: 1952



@dwisiswant0: Done! The issue has been created at #7483[BUG] krbforge JS bindings do not enforce sandbox policy for ccache file writes.

@Mzack9999
Mzack9999 merged commit 3a8b65c into dev Jun 23, 2026
19 checks passed
@Mzack9999
Mzack9999 deleted the dwisiswant0/fix/js/reject-unsandboxed-ccache-writes-in-krbforge branch June 23, 2026 20:36
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.

[BUG] krbforge JS bindings do not enforce sandbox policy for ccache file writes

2 participants