Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
144 changes: 142 additions & 2 deletions pkg/js/compiler/compiler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"path/filepath"
"runtime"
"strings"
"sync"
"testing"
"time"

Expand Down Expand Up @@ -94,17 +95,59 @@ func TestRequireDoesNotReusePrivilegedModuleCacheAcrossExecutions(t *testing.T)
runtime := createNewRuntime()
firstValue, err := executeWithRuntime(t.Context(), runtime, program, NewExecuteArgs(), &ExecuteOptions{
ExecutionId: allowExecutionID,
})
}, nil)
require.NoError(t, err)
require.Equal(t, "outside-ok", firstValue.Export())

_, err = executeWithRuntime(t.Context(), runtime, program, NewExecuteArgs(), &ExecuteOptions{
ExecutionId: denyExecutionID,
})
}, nil)
require.Error(t, err)
require.Contains(t, err.Error(), "-lfa is not enabled")
}

func TestExecuteWithRuntimeCleansUpAfterCallbackPanic(t *testing.T) {
program, err := goja.Compile("", `1`, false)
require.NoError(t, err)

runtime := createNewRuntime()
args := NewExecuteArgs()
args.Args["arg"] = "value"
args.TemplateCtx["template-key"] = "template-value"

cleanupCalled := false
panicValue := "callback panic"

func() {
defer func() {
require.Equal(t, panicValue, recover())
}()

_, _ = executeWithRuntime(t.Context(), runtime, program, args, &ExecuteOptions{
ExecutionId: "callback-panic-cleanup",
Callback: func(rt *goja.Runtime) error {
require.NoError(t, rt.Set("callbackState", "partial"))
panic(panicValue)
},
Cleanup: func(rt *goja.Runtime) {
cleanupCalled = true
_ = rt.GlobalObject().Delete("callbackState")
},
}, nil)
t.Fatal("executeWithRuntime did not panic")
}()

require.True(t, cleanupCalled)
require.Nil(t, runtime.Get("template"))
require.Nil(t, runtime.Get("arg"))
require.Nil(t, runtime.Get("callbackState"))

_, ok := runtime.GetContextValue("executionId")
require.False(t, ok)
_, ok = runtime.GetContextValue("ctx")
require.False(t, ok)
}

func TestNonPooledRuntimeTerminatesOnContextExpiry(t *testing.T) {
timeout := 300 * time.Millisecond

Expand Down Expand Up @@ -153,6 +196,103 @@ func TestPooledRuntimeTerminatesOnContextExpiry(t *testing.T) {
}
}

// TestPooledRuntimeAbandonedReleasesSlotOnlyAfterOrphanExits verifies that
// when a runtime is abandoned because its goja goroutine outlived the
// interrupt grace period, executeWithPoolingProgram does NOT release the
// concurrency slot eagerly. Releasing it eagerly would let a stream of
// stuck callbacks bypass PoolingJsVmConcurrency and trade the original
// map-race crash for unbounded resource growth. The slot must stay held
// by the reaper goroutine until the orphan goroutine actually exits.
func TestPooledRuntimeAbandonedReleasesSlotOnlyAfterOrphanExits(t *testing.T) {
src := `ExportAs("k","v"); block(); 1`
p, err := SourceAutoMode(src, false)
require.NoError(t, err)

release := make(chan struct{})
var releaseOnce sync.Once
releaseOrphan := func() { releaseOnce.Do(func() { close(release) }) }
t.Cleanup(releaseOrphan)

lazySgInit()
initialCurrent := pooljsc.Current()

ctx, cancel := context.WithTimeout(t.Context(), 100*time.Millisecond)
defer cancel()

_, err = executeWithPoolingProgram(ctx, p, NewExecuteArgs(), &ExecuteOptions{
Source: &src,
Callback: func(rt *goja.Runtime) error {
return rt.Set("block", func() {
<-release
})
},
})
require.Error(t, err)
require.ErrorIs(t, err, errRuntimeTerminationTimeout,
"runtime should be flagged as abandoned when its goroutine outlives the interrupt grace period")

require.GreaterOrEqual(t, pooljsc.Current(), initialCurrent+1,
"abandoned runtime must keep its concurrency slot held by the reaper while the orphan is still alive")

releaseOrphan()

require.Eventually(t, func() bool {
return pooljsc.Current() == initialCurrent
}, 5*time.Second, 10*time.Millisecond, "concurrency slot must be released once the orphan goroutine exits")
}

// TestPooledRuntimeAbandonedWhenInterruptStuck reproduces the conditions
// of https://github.com/projectdiscovery/nuclei/issues/7376: a JS program
// is blocked inside a native Go call and ignores the goja Interrupt(), so
// the inner goroutine outlives the grace period after a context cancel.
// Before the fix, the runtime would still be returned to the sync.Pool and
// reused by the next caller, which would race with the orphan goroutine
// on goja's per-runtime maps and trigger a fatal "concurrent map read and
// map write" runtime panic. After the fix, executeWithPoolingProgram must
// return errRuntimeTerminationTimeout (still wrapping context.DeadlineExceeded
// for backwards compatibility) and abandon the runtime instead of pooling it.
func TestPooledRuntimeAbandonedWhenInterruptStuck(t *testing.T) {
// A real script using ExportAs so executeWithPoolingProgram (not the
// non-pooled fallback) is taken — see ExecuteProgram source-routing.
src := `ExportAs("k", "v"); block(); 1`
p, err := SourceAutoMode(src, false)
require.NoError(t, err)

ctx, cancel := context.WithTimeout(t.Context(), 100*time.Millisecond)
defer cancel()

// The native Go function blocks until release is closed. We close it
// only after the test has observed the timeout error so the orphan
// goroutine eventually unwinds and the abandoned runtime is GC-able.
release := make(chan struct{})
t.Cleanup(func() { close(release) })

done := make(chan error, 1)
go func() {
_, err := executeWithPoolingProgram(ctx, p, NewExecuteArgs(), &ExecuteOptions{
Source: &src,
Callback: func(rt *goja.Runtime) error {
return rt.Set("block", func() {
<-release
})
},
})
done <- err
}()

// 100ms ctx + 1s goja interrupt grace period + slack
select {
case err := <-done:
require.Error(t, err)
require.ErrorIs(t, err, errRuntimeTerminationTimeout,
"runtime should be flagged as abandoned when its goroutine outlives the interrupt grace period")
require.ErrorIs(t, err, context.DeadlineExceeded,
"the underlying context cancellation cause should still be reachable for callers using errors.Is")
case <-time.After(5 * time.Second):
t.Fatal("executeWithPoolingProgram did not return within the grace period")
}
}

func executeScript(t *testing.T, executionID string, allowLocalFileAccess bool, script string) (ExecuteResult, error) {
t.Helper()
protocolstate.SetLfaAllowed(&types.Options{ExecutionId: executionID, AllowLocalFileAccess: allowLocalFileAccess})
Expand Down
20 changes: 18 additions & 2 deletions pkg/js/compiler/non-pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package compiler

import (
"context"
"errors"
"sync"

"github.com/Mzack9999/goja"
Expand All @@ -22,8 +23,23 @@ func executeWithoutPooling(ctx context.Context, p *goja.Program, args *ExecuteAr
if err := ephemeraljsc.AddWithContext(ctx); err != nil {
return nil, err
}
defer ephemeraljsc.Done()
// When the runtime is abandoned (an orphan goroutine outlived the
// interrupt grace period) ownership of the concurrency slot is
// transferred to a reaper inside executeWithRuntime which calls
// ephemeraljsc.Done after the orphan goroutine eventually exits;
// releasing the slot eagerly would let stuck callbacks bypass
// NonPoolingVMConcurrency.
slotOwnedByReaper := false
defer func() {
if !slotOwnedByReaper {
ephemeraljsc.Done()
}
}()

runtime := createNewRuntime()
return executeWithRuntime(ctx, runtime, p, args, opts)
val, runErr := executeWithRuntime(ctx, runtime, p, args, opts, ephemeraljsc.Done)
if errors.Is(runErr, errRuntimeTerminationTimeout) {
slotOwnedByReaper = true
}
return val, runErr
}
Loading
Loading