Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 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
27 changes: 27 additions & 0 deletions cmd/integration-test/javascript.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ var jsTestcases = []TestCaseInfo{
{Path: "protocols/javascript/net-https.yaml", TestCase: &javascriptNetHttps{}},
{Path: "protocols/javascript/oracle-auth-test.yaml", TestCase: &javascriptOracleAuthTest{}, DisableOn: func() bool { return osutils.IsWindows() || osutils.IsOSX() }},
{Path: "protocols/javascript/vnc-pass-brute.yaml", TestCase: &javascriptVncPassBrute{}},
{Path: "protocols/javascript/multi-ports.yaml", TestCase: &javascriptMultiPortsSSH{}},
}

var (
Expand Down Expand Up @@ -167,6 +168,32 @@ func (j *javascriptVncPassBrute) Execute(filePath string) error {
return multierr.Combine(errs...)
}

type javascriptMultiPortsSSH struct{}

func (j *javascriptMultiPortsSSH) Execute(filePath string) error {
finalURL := "scanme.sh"
errs := []error{}
for i := 0; i < defaultRetry; i++ {
results := []string{}
var err error
_ = pool.Retry(func() error {
//let ssh server start
time.Sleep(3 * time.Second)
results, err = testutils.RunNucleiTemplateAndGetResults(filePath, finalURL, debug)
return nil
})
if err != nil {
return err
}
if err := expectResultsCount(results, 1); err == nil {
return nil
} else {
errs = append(errs, err)
}
}
return multierr.Combine(errs...)
}
Comment on lines +173 to +180

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Multiple issues with executor implementation.

The javascriptMultiPortsSSH executor has several problems that deviate from the established pattern:

  1. Line 175: Checks sshResource instead of multiPortsSShResource, which creates inconsistency with the declared variable at line 28.

  2. Line 176: Copy-paste error in comment—says "redis" but should say "SSH" or "multi-port SSH".

  3. Line 179: Uses hardcoded "scanme.sh" instead of extracting a port from a Docker resource like all other executors (see lines 50-51, 82-83, 114-115, 147-148). This breaks the established pattern of testing against local Docker containers.

  4. Missing defer purge(): Unlike all other executors (lines 52, 84, 116, 149), this executor doesn't clean up the Docker resource, potentially causing resource leaks.

Apply this diff to align with the established pattern (assuming you want to use the existing sshResource):

 func (j *javascriptMultiPortsSSH) Execute(filePath string) error {
 	if sshResource == nil || pool == nil {
-		// skip test as redis is not running
+		// skip test as ssh is not running
 		return nil
 	}
-	finalURL := "scanme.sh"
+	tempPort := sshResource.GetPort("2222/tcp")
+	finalURL := "localhost:" + tempPort
+	defer purge(sshResource)
 	errs := []error{}

Alternatively, if the test genuinely needs to target the external scanme.sh service (which seems inconsistent with an integration test suite using Docker), please clarify the intent and consider whether this should be a separate test type.

Comment on lines +171 to +180

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Avoid using pool.Retry here – risk of nil-pointer panic and unnecessary Docker dependency

javascriptMultiPortsSSH.Execute currently calls pool.Retry but:

  • There is no pool == nil guard, unlike the other executors. If Docker is unavailable or init() fails, the global pool can be nil, leading to a panic when pool.Retry is called.
  • This executor does not use any Docker resource at all and only targets scanme.sh, so it doesn’t actually need the dockertest pool; the retry loop can be implemented directly.
  • The comment //let ssh server start is misleading here, since there is no local SSH container involved.

To make this robust and simpler, you can drop the pool dependency entirely and keep the retry behavior, e.g.:

 func (j *javascriptMultiPortsSSH) Execute(filePath string) error {
-	finalURL := "scanme.sh"
-	errs := []error{}
-	for i := 0; i < defaultRetry; i++ {
-		results := []string{}
-		var err error
-		_ = pool.Retry(func() error {
-			//let ssh server start
-			time.Sleep(3 * time.Second)
-			results, err = testutils.RunNucleiTemplateAndGetResults(filePath, finalURL, debug)
-			return nil
-		})
-		if err != nil {
-			return err
-		}
-		if err := expectResultsCount(results, 1); err == nil {
-			return nil
-		} else {
-			errs = append(errs, err)
-		}
-	}
-	return multierr.Combine(errs...)
+	finalURL := "scanme.sh"
+	errs := []error{}
+	for i := 0; i < defaultRetry; i++ {
+		// give remote endpoint a brief pause between attempts
+		time.Sleep(3 * time.Second)
+
+		results, err := testutils.RunNucleiTemplateAndGetResults(filePath, finalURL, debug)
+		if err != nil {
+			return err
+		}
+		if err := expectResultsCount(results, 1); err == nil {
+			return nil
+		}
+		errs = append(errs, err)
+	}
+	return multierr.Combine(errs...)
 }

This removes the potential nil-pointer on pool, keeps the retry logic, and avoids binding this test to Docker when it only needs a remote host.

🤖 Prompt for AI Agents
In cmd/integration-test/javascript.go around lines 171 to 195, replace the use
of pool.Retry (which can panic if pool is nil and is unnecessary for a
remote-only test) with a direct retry loop: remove the pool.Retry call and the
misleading "//let ssh server start" comment, perform up to defaultRetry
iterations where you sleep briefly (e.g., 3s) before calling
testutils.RunNucleiTemplateAndGetResults(filePath, finalURL, debug), capture its
error into a local err variable, check expectResultsCount and return on success,
otherwise append the error to errs; after the loop, return
multierr.Combine(errs...) so the behavior and backoff remain but without
depending on pool or Docker.


// purge any given resource if it is not nil
func purge(resource *dockertest.Resource) {
if resource != nil && pool != nil {
Expand Down
1 change: 0 additions & 1 deletion lib/example_test.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
//go:build !race
// +build !race

package nuclei_test

Expand Down
2 changes: 1 addition & 1 deletion pkg/operators/common/dsl/dsl.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ func init() {
}))

dsl.PrintDebugCallback = func(args ...interface{}) error {
gologger.Debug().Msgf("print_debug value: %s", fmt.Sprint(args))
gologger.Debug().Msgf("print_debug value: %s", fmt.Sprint(args)) //nolint
return nil
}

Expand Down
40 changes: 34 additions & 6 deletions pkg/protocols/javascript/js.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import (
"github.com/projectdiscovery/utils/errkit"
iputil "github.com/projectdiscovery/utils/ip"
mapsutil "github.com/projectdiscovery/utils/maps"
sliceutil "github.com/projectdiscovery/utils/slice"
syncutil "github.com/projectdiscovery/utils/sync"
urlutil "github.com/projectdiscovery/utils/url"
)
Expand Down Expand Up @@ -133,8 +134,11 @@ func (request *Request) Compile(options *protocols.ExecutorOptions) error {
}

// "Port" is a special variable and it should not contains any dsl expressions
if strings.Contains(request.getPort(), "{{") {
return errkit.New("'Port' variable cannot contain any dsl expressions")
ports := request.getPorts()
for _, port := range ports {
if strings.Contains(port, "{{") {
return errkit.New("'Port' variable cannot contain any dsl expressions")
}
}

if request.Init != "" {
Expand Down Expand Up @@ -281,12 +285,28 @@ func (request *Request) GetID() string {

// ExecuteWithResults executes the protocol requests and returns results instead of writing them.
func (request *Request) ExecuteWithResults(target *contextargs.Context, dynamicValues, previous output.InternalEvent, callback protocols.OutputEventCallback) error {
// Get default port(s) if specified in template
ports := request.getPorts()

var errs []error

for _, port := range ports {
err := request.executeWithResults(port, target, dynamicValues, previous, callback)
if err != nil {
errs = append(errs, err)
}
}

return errkit.Join(errs...)
}
Comment on lines +288 to +301

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Critical: Templates without Port won't execute.

When getPorts() returns an empty slice (no Port specified in template args), the loop doesn't execute and the function returns immediately. This breaks existing JavaScript templates that don't specify a Port argument.

Apply this diff to execute once with an empty port when no ports are specified:

 func (request *Request) ExecuteWithResults(target *contextargs.Context, dynamicValues, previous output.InternalEvent, callback protocols.OutputEventCallback) error {
 	// Get default port(s) if specified in template
 	ports := request.getPorts()
+	if len(ports) == 0 {
+		ports = []string{""}
+	}
 
 	for _, port := range ports {
 		err := request.executeWithResults(port, target, dynamicValues, previous, callback)
 		if err != nil {
 			return err
 		}
 	}
 
 	return nil
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Get default port(s) if specified in template
ports := request.getPorts()
for _, port := range ports {
err := request.executeWithResults(port, target, dynamicValues, previous, callback)
if err != nil {
return err
}
}
return nil
}
func (request *Request) ExecuteWithResults(target *contextargs.Context, dynamicValues, previous output.InternalEvent, callback protocols.OutputEventCallback) error {
// Get default port(s) if specified in template
ports := request.getPorts()
if len(ports) == 0 {
ports = []string{""}
}
for _, port := range ports {
err := request.executeWithResults(port, target, dynamicValues, previous, callback)
if err != nil {
return err
}
}
return nil
}
🤖 Prompt for AI Agents
In pkg/protocols/javascript/js.go around lines 288 to 299, the current logic
returns immediately when request.getPorts() yields an empty slice, which
prevents templates without a Port arg from running; change the flow so that if
ports is empty you call request.executeWithResults once with an empty-string (or
zero-value) port (preserving the same target, dynamicValues, previous, callback)
and return any error; otherwise keep iterating over ports and return any error
from each call. Ensure you do not alter error handling semantics and that a
single execution occurs when no ports are specified.


// executeWithResults executes the request
func (request *Request) executeWithResults(port string, target *contextargs.Context, dynamicValues, previous output.InternalEvent, callback protocols.OutputEventCallback) error {
input := target.Clone()
// use network port updates input with new port requested in template file
// and it is ignored if input port is not standard http(s) ports like 80,8080,8081 etc
// idea is to reduce redundant dials to http ports
if err := input.UseNetworkPort(request.getPort(), request.getExcludePorts()); err != nil {
if err := input.UseNetworkPort(port, request.getExcludePorts()); err != nil {
gologger.Debug().Msgf("Could not network port from constants: %s\n", err)
}

Expand Down Expand Up @@ -755,13 +775,21 @@ func (request *Request) Type() templateTypes.ProtocolType {
return templateTypes.JavascriptProtocol
}

func (request *Request) getPort() string {
func (request *Request) getPorts() []string {
for k, v := range request.Args {
if strings.EqualFold(k, "Port") {
return types.ToString(v)
portStr := types.ToString(v)
ports := []string{}
for _, p := range strings.Split(portStr, ",") {
trimmed := strings.TrimSpace(p)
if trimmed != "" {
ports = append(ports, trimmed)
}
}
return sliceutil.Dedupe(ports)
}
}
return ""
return []string{}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

func (request *Request) getExcludePorts() string {
Expand Down
1 change: 0 additions & 1 deletion pkg/scan/events/scan_noop.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
//go:build !stats
// +build !stats

package events

Expand Down
1 change: 0 additions & 1 deletion pkg/scan/events/stats_build.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
//go:build stats
// +build stats

package events

Expand Down
2 changes: 0 additions & 2 deletions pkg/utils/json/json.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
//go:build (linux || darwin || windows) && (amd64 || arm64)
// +build linux darwin windows
// +build amd64 arm64

package json

Expand Down
1 change: 0 additions & 1 deletion pkg/utils/json/json_fallback.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
//go:build !(linux || darwin || windows) || !(amd64 || arm64)
// +build !linux,!darwin,!windows !amd64,!arm64

package json

Expand Down