Skip to content

fix: ensure connection cleanup in JARM fingerprint loop #2545

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
lxcxjxhx wants to merge 1 commit into from
+20 −15
Closed

fix: ensure connection cleanup in JARM fingerprint loop #2545

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
35 changes: 20 additions & 15 deletions common/hashes/jarm/jarmhash.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,21 +49,26 @@ func fingerprint(dialer *fastdialer.Dialer, t target, duration int) string {
if conn == nil {
return ""
}
_ = conn.SetWriteDeadline(time.Now().Add(timeout))
_, err = conn.Write(jarm.BuildProbe(probe))
if err != nil {
_ = conn.Close()
return ""
}
_ = conn.SetReadDeadline(time.Now().Add(timeout))
buff := make([]byte, 1484)
_, _ = conn.Read(buff)
_ = conn.Close()
ans, err := jarm.ParseServerHello(buff, probe)
if err != nil {
return ""
}
results = append(results, ans)

func() {
defer conn.Close()

@coderabbitai coderabbitai Bot Jul 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Handle the deferred Close error.

Line [54] triggers errcheck. Since fingerprint cannot propagate cleanup errors, explicitly discard the result or add a narrowly scoped lint exemption.

Proposed fix 
-			defer conn.Close()
+			defer func() {
+				_ = conn.Close()
+			}()
📝 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
defer conn.Close()
defer func() {
_ = conn.Close()
}()
🧰 Tools
🪛 golangci-lint (2.12.2)

[error] 54-54: Error return value of conn.Close is not checked

(errcheck)

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@common/hashes/jarm/jarmhash.go` at line 54, Update the deferred connection
cleanup in fingerprint to explicitly discard the result of conn.Close, resolving
the errcheck warning without changing fingerprint’s error behavior.

Source: Linters/SAST tools


_ = conn.SetWriteDeadline(time.Now().Add(timeout))
_, err = conn.Write(jarm.BuildProbe(probe))
if err != nil {
return
}
_ = conn.SetReadDeadline(time.Now().Add(timeout))
buff := make([]byte, 1484)
if _, err = conn.Read(buff); err != nil {
return
}
ans, err := jarm.ParseServerHello(buff, probe)
Comment on lines +63 to +66

@coderabbitai coderabbitai Bot Jul 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n -C3 'ParseServerHello|\.Read\(' common/hashes/jarm --glob '*.go'

Repository: projectdiscovery/httpx

Length of output: 734

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== file outline =="
ast-grep outline common/hashes/jarm/jarmhash.go || true

echo
echo "== jarmhash.go relevant sections =="
sed -n '1,140p' common/hashes/jarm/jarmhash.go

echo
echo "== parse server hello implementation/usages =="
rg -n -C4 'func ParseServerHello|ParseServerHello\(' common --glob '*.go'

echo
echo "== any connection wrappers/readers in common/hashes/jarm =="
rg -n -C3 'Read\(|conn.*Read|Read\(' common/hashes/jarm --glob '*.go'

Repository: projectdiscovery/httpx

Length of output: 3543

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Acquire/OneTimePool definition and implementations =="
rg -n -C5 'func newOneTimePool|type .*OneTime|func \(.*\) Acquire|type .*Connection|type .*Conn' common --glob '*.go'

echo
echo "== imports and pool files =="
fd -e go . common/hashes/jarm -x sh -c 'echo "--- $1"; sed -n "1,160p" "$1"' sh {}

Repository: projectdiscovery/httpx

Length of output: 6673

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== fastdialer Dial implementations and net.Conn wrappers =="
rg -n -C5 'func \(.*\) Dial|type .*Dial|Dial\(ctx|net.Conn|Read\(.*error' -g '*.go' .

Repository: projectdiscovery/httpx

Length of output: 6826

🌐 Web query:

hdm jarm-go ParseServerHello buffer length expected

💡 Result:

In the context of the jarm-go library (widely used for JARM fingerprinting), the ParseServerHello function is responsible for parsing a TLS ServerHello message from raw byte data [1][2][3]. The error regarding "buffer length expected" or similar validation failures typically arises when the provided byte buffer is too short to contain a valid TLS ServerHello structure [4]. JARM probing relies on receiving a response from a server, which is then passed to the parsing logic [1][3]. Key technical context: 1. Expected Data: The function expects a byte array containing a valid TLS handshake response. If the connection received fewer bytes than required by the TLS protocol specification (or the specific implementation's parser), an error will be returned [4]. 2. JARM Context: When using jarm-go, the library typically receives up to 1484 bytes from the server [1][5]. If the network read is interrupted or the server sends an incomplete response, the buffer passed to ParseServerHello will not meet the minimum length requirements for header fields, resulting in a parsing error [1][4]. 3. Troubleshooting: If you are encountering this error, ensure that the data being passed into ParseServerHello is actually a complete TLS ServerHello handshake message. In many implementations, the data is expected to start with the TLS record header (e.g., 5 bytes for the record layer) followed by the handshake message [6][4]. If you are writing custom code or debugging, verify that your read operation has fully buffered the expected amount of data before attempting to parse it [4].

Citations:

Preserve bytes returned with a read error.

conn.Read implements Go’s io.Reader contract, which allows n > 0 with a non-nil error. Returning immediately can discard a partial JARM response that ParseServerHello is still able to parse. Capture n and pass buff[:n] into ParseServerHello; only return when no bytes were read.

Suggested adjustment 
-			if _, err = conn.Read(buff); err != nil {
+			n, readErr := conn.Read(buff)
+			if n == 0 && readErr != nil {
 				return
 			}
-			ans, err := jarm.ParseServerHello(buff, probe)
+			ans, err := jarm.ParseServerHello(buff[:n], probe)
📝 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
if _, err = conn.Read(buff); err != nil {
return
}
ans, err := jarm.ParseServerHello(buff, probe)
n, readErr := conn.Read(buff)
if n == 0 && readErr != nil {
return
}
ans, err := jarm.ParseServerHello(buff[:n], probe)
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@common/hashes/jarm/jarmhash.go` around lines 63 - 66, Update the conn.Read
handling in the JARM probe flow to capture the returned byte count and preserve
partial data when an error accompanies n > 0. Pass buff[:n] to
jarm.ParseServerHello, and return immediately only when no bytes were read;
retain the existing error behavior otherwise.

if err != nil {
return
}
results = append(results, ans)
}()
}
return jarm.RawHashToFuzzyHash(strings.Join(results, ","))
}
Expand Down