Skip to content

fix(js): respect network policy before dialing ldap - #7494

Merged
Mzack9999 merged 1 commit into
devfrom
dwisiswant0/fix/js/respect-network-policy-before-dialing-ldap
Jun 23, 2026
Merged

Mzack9999 merged 1 commit into
devfrom
dwisiswant0/fix/js/respect-network-policy-before-dialing-ldap

Conversation

@dwisiswant0

@dwisiswant0 dwisiswant0 commented Jun 22, 2026 •

Copy link
Copy Markdown
Member

Proposed changes

LDAP was the only JS protocol client that reached
the dialer without an IsHostAllowed check.
Validate ldap, ldaps, and cldap hosts before
connecting, and treat ldapi as loopback so
restricted local network access rejects Unix
socket LDAP before the socket path is opened.

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

    • Enhanced network policy enforcement for LDAP connections, ensuring restricted local network access is properly validated before connection attempts are made.
  • Tests

    • Added comprehensive test coverage for network policy validation in LDAP connection scenarios.

LDAP was the only JS protocol client that reached
the dialer without an `IsHostAllowed` check.
Validate ldap, ldaps, and cldap hosts before
connecting, and treat ldapi as loopback so
restricted local network access rejects Unix
socket LDAP before the socket path is opened.

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

coderabbitai Bot commented Jun 22, 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: 31e12eaf-b484-48fc-b266-5c623f88d3c5

📥 Commits

Reviewing files that changed from the base of the PR and between 323df47 and 0a2ed8f.

📒 Files selected for processing (2)
  • pkg/js/libs/ldap/ldap.go
  • pkg/js/libs/ldap/ldap_test.go

Walkthrough

NewClient in the LDAP library is refactored to enforce protocolstate.IsHostAllowed allowlist checks before dialing for all schemes: ldapi checks against a fixed 127.0.0.1 policy host, while ldap/cldap/ldaps derive and validate the host first. Scheme defaulting is moved earlier. Three new tests verify denial and allowance behavior.

Changes

LDAP Client Network Policy Enforcement

Layer / File(s) Summary
NewClient scheme defaulting and allowlist enforcement
pkg/js/libs/ldap/ldap.go
Imports reordered; u.Scheme now defaults to "ldap" immediately after url.Parse; dialing branch refactored so ldapi checks protocolstate.IsHostAllowed against 127.0.0.1 and ldap/cldap/ldaps extract host via u.Hostname(), validate non-empty, then check IsHostAllowed before dialing.
Network policy tests and helpers
pkg/js/libs/ldap/ldap_test.go
Adds TestNewClientDeniesRestrictedLocalTCPBeforeDial and TestNewClientDeniesLDAPIAsLocalNetworkAccess asserting errors contain "network policy" before any dial; TestNewClientAllowsTCPWhenNetworkPolicyAllows verifies a local listener receives a connection; helper functions manage protocolstate lifecycle and centralize policy-denial assertions.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 A bunny guards the LDAP gate,
No sneaky socket slips through late!
IsHostAllowed checks the list,
ldapi and ldap — none are missed.
The tests confirm: deny before dial,
Secure connections, hop-by-hop style! 🔒

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 14.29% 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 accurately summarizes the main change: implementing network policy validation before LDAP connections are established.
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/respect-network-policy-before-dialing-ldap

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

Comment thread pkg/js/libs/ldap/ldap.go Dismissed
Comment thread pkg/js/libs/ldap/ldap.go Dismissed
@dwisiswant0
dwisiswant0 requested a review from Mzack9999 June 23, 2026 02:31
@dwisiswant0

Copy link
Copy Markdown
Member Author

@coderabbitai create an issue for this.

@coderabbitai

coderabbitai Bot commented Jun 23, 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 "LDAP JS client missing IsHostAllowed check — network policy bypass" \
  --assignee "dwisiswant0" \
  --body "## Summary

The LDAP JavaScript protocol client in \`pkg/js/libs/ldap/ldap.go\` was the only JS protocol client that could reach the dialer without an \`IsHostAllowed\` check. This meant network access restrictions (\`RestrictLocalNetworkAccess\` and similar policies) could be bypassed when a template used the LDAP client.

## Details

- **Affected schemes:** \`ldap://\`, \`ldaps://\` (LDAP over TLS), \`cldap://\` (connectionless LDAP), \`ldapi://\` (LDAP over Unix sockets)
- **Root cause:** \`NewClient\` in \`pkg/js/libs/ldap/ldap.go\` did not call \`protocolstate.IsHostAllowed\` before dialing, unlike all other JS protocol clients.
- **Impact:** A nuclei template using the LDAP client could establish connections to hosts that should be blocked by the configured network policy.

## Expected behaviour

Connections to restricted hosts/networks via the LDAP client should be denied with a network-policy error before any dialing attempt, consistent with other protocol clients.

## Fix (implemented in `#7494`)

- For \`ldap\`, \`ldaps\`, and \`cldap\`: derive the target host and call \`protocolstate.IsHostAllowed\` before dialing.
- For \`ldapi\` (Unix sockets): treat the connection as loopback (\`127.0.0.1\`) so that \`RestrictLocalNetworkAccess\` policies correctly reject it before the socket path is opened.

## References

- PR: https://github.com/projectdiscovery/nuclei/pull/7494
- Reported by: `@dwisiswant0`"

Length of output: 1828



@dwisiswant0, the issue has been created and assigned to you: #7496 🐇

@Mzack9999
Mzack9999 merged commit 9def9af into dev Jun 23, 2026
19 checks passed
@Mzack9999
Mzack9999 deleted the dwisiswant0/fix/js/respect-network-policy-before-dialing-ldap branch June 23, 2026 20:24
social4hyq pushed a commit to social4hyq/homebrew-core that referenced this pull request Sep 20, 2026
nuclei 3.10.0

Created-by: HarmonybrewBot
Commit-by: HarmonybrewBot
Merged-by: HarmonybrewBot
Description: Created by `brew bump`

---

Created with `brew bump-formula-pr`.<details>
  <summary>release notes</summary>
  <pre>## What's Changed

### 🎉 New Features
* Added per-host HTTP client pooling by @Mzack9999 in projectdiscovery/nuclei#7301

### 🐞 Bug Fixes
* Fixed handling in hosterrorscache to automatically skip hosts that consistently time out by @knakul853 in projectdiscovery/nuclei#7455
* Fixed preservation of explicit target port in network templates (fixes #7323) by @XananasX7 in projectdiscovery/nuclei#7465
* Fixed connection reuse and improved port pre-flight handling by @Mzack9999 in projectdiscovery/nuclei#6715
* Fixed code template signature validation before DAST loading by @dogancanbakir in projectdiscovery/nuclei#7472
* Fixed gating of MySQL allowAllFiles option to require `-lfa` flag by @dogancanbakir in projectdiscovery/nuclei#7473
* Fixed ASCII-section regex to properly escape literal `.` by @snicket2100 in projectdiscovery/nuclei#7476
* Fixed recording of decoded bytes for debug dumps by @snicket2100 in projectdiscovery/nuclei#7478
* Fixed proper escaping of dbname in lib/pq URLs by @dwisiswant0 in projectdiscovery/nuclei#7479
* Fixed network policy enforcement prior to LDAP dialing by @dwisiswant0 in projectdiscovery/nuclei#7494
* Fixed normalization and rejection of trace file DSN options in Oracle by @dwisiswant0 in projectdiscovery/nuclei#7480
* Fixed proper escaping of MSSQL database names in connection URLs by @dwisiswant0 in projectdiscovery/nuclei#7481
* Fixed krbforge to reject unsandboxed ccache writes by @dwisiswant0 in projectdiscovery/nuclei#7482
* Fixed rejection of request-condition(s) during fuzzing by @dwisiswant0 in projectdiscovery/nuclei#7466
* Fixed: YAML now correctly rejects recursive include chains by @dwisiswant0 in projectdiscovery/nuclei#7492
* Fixed resource leaks by @Mzack9999 in projectdiscovery/nuclei#7502

### Other Changes
* Updated govaluate dependency to prevent slice-bounds panic on invalid UTF-8 input by @XananasX7 in projectdiscovery/nuclei#7464
* Updated goja dependency by @Mzack9999 in projectdiscovery/nuclei#7467
* Updated dependencies to remove unused packages by @dwisiswant0 in projectdiscovery/nuclei#7457
* Refactored templates to centralize opt-in capability gating by @dwisiswant0 in projectdiscovery/nuclei#7489
* Added fuzzing parser harnesses for raw requests and templates by @dwisiswant0 in projectdiscovery/nuclei#7459
* Refactored template rendering boundary by @dwisiswant0 in projectdiscovery/nuclei#7499

## New Contributors
* @XananasX7 made their first contribution in projectdiscovery/nuclei#7465
* @snicket2100 made their first contribution in projectdiscovery/nuclei#7476

**Full Changelog**: https://github.com/projectdiscovery/nuclei/compare/v3.9.0...v3.10.0</pre>
  <p>View the full release notes at <a href="https://github.com/projectdiscovery/nuclei/releases/tag/v3.10.0">https://github.com/projectdiscovery/nuclei/releases/tag/v3.10.0</a>.</p>
</details>
<hr>

See merge request: Harmonybrew/homebrew-core!13187
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] LDAP JS client missing IsHostAllowed check — network policy bypass

3 participants