http1: fix potential NULL dereference in `Curl_h1_req_parse_read()` by vszakats · Pull Request #20779 · curl/curl

vszakats · 2026-03-01T04:01:56Z

Reported by clang-tidy v22 with clang-analyzer-* explicitly enabled:

lib/http1.c:89:31: error: Subtraction of a non-null pointer
 (from variable 'line_end') and a null pointer (via field 'line')
 results in undefined behavior [clang-analyzer-core.NullPointerArithm]
   89 |   parser->line_len = line_end - parser->line + 1;
      |                               ^

Ref: https://github.com/curl/curl/actions/runs/22534731241/job/65279952830?pr=20778#step:11:85

Ref: #20778

vszakats · 2026-03-01T04:03:28Z

I'm not sure if this is the correct thing to do when a NULL buffer
is received in this function. Maybe it should return a CURLE error?

bagder · 2026-03-01T11:54:46Z

I think it should primarily have an assert for it so that a debug build would crash on it because it is a fundamental requirement that this function is never called with a NULL pointer and I don't see how this can actually legitimately happen.

Then it becomes a mostly academic question how to handle it in run-time. It should probably return an error because it is a sign of a serious internal confusion.

bagder · 2026-03-02T08:19:35Z

augment review

augmentcode · 2026-03-02T08:20:45Z

🤖 Augment PR Summary

Summary: Adds a defensive DEBUGASSERT(buf) and a runtime NULL check in Curl_h1_req_parse_read() to avoid undefined behavior during HTTP/1 request parsing.

Why: Addresses a clang-analyzer finding that the function could be analyzed as doing pointer arithmetic on a NULL buffer.

_{🤖 Was this summary useful? React with 👍 or 👎}

augmentcode

Review completed. 1 suggestions posted.

Comment augment review to trigger a new review at any time.

lib/http1.c

vszakats · 2026-03-02T12:09:25Z

@aisle-analyzer augment review

aisle-research-bot · 2026-03-02T12:09:33Z

🔒 Aisle Security Analysis

✅ We scanned this PR and did not find any security vulnerabilities.

_{Aisle supplements but does not replace security review.}

Analyzed PR: #20779 at commit f5465d2

_{Last updated on: 2026-03-02T12:33:36Z}

Reported by clang-tidy v22 with `clang-analyzer-*` explicitly enabled. ``` lib/http1.c:89:31: error: Subtraction of a non-null pointer (from variable 'line_end') and a null pointer (via field 'line') results in undefined behavior [clang-analyzer-core.NullPointerArithm,-warnings-as-errors] 89 | parser->line_len = line_end - parser->line + 1; | ^ lib/http1.c:272:9: note: Assuming field 'done' is 0 272 | while(!parser->done) { | ^~~~~~~~~~~~~ lib/http1.c:272:3: note: Loop condition is true. Entering loop body 272 | while(!parser->done) { | ^ lib/http1.c:273:14: note: Calling 'next_line' 273 | result = next_line(parser, buf, buflen, options, &nread); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ lib/http1.c:101:6: note: Assuming field 'line' is null 101 | if(parser->line) { | ^~~~~~~~~~~~ lib/http1.c:101:3: note: Taking false branch 101 | if(parser->line) { | ^ lib/http1.c:107:12: note: Calling 'detect_line' 107 | result = detect_line(parser, buf, buflen, pnread); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ lib/http1.c:83:3: note: Loop condition is false. Exiting loop 83 | DEBUGASSERT(!parser->line); | ^ lib/curl_setup.h:1072:24: note: expanded from macro 'DEBUGASSERT' 1072 | #define DEBUGASSERT(x) do {} while(0) | ^ lib/http1.c:86:6: note: Assuming 'line_end' is non-null 86 | if(!line_end) | ^~~~~~~~~ lib/http1.c:86:3: note: Taking false branch 86 | if(!line_end) | ^ lib/http1.c:88:3: note: The value of 'buf' is assigned to field 'line', which participates in a condition later 88 | parser->line = (const char *)buf; | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ lib/http1.c:89:3: note: Value assigned to field 'line_len' 89 | parser->line_len = line_end - parser->line + 1; | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ lib/http1.c:90:3: note: Value assigned to 'nread' 90 | *pnread = parser->line_len; | ^~~~~~~~~~~~~~~~~~~~~~~~~~ lib/http1.c:90:3: note: Value assigned to 'nread', which participates in a condition later 90 | *pnread = parser->line_len; | ^~~~~~~~~~~~~~~~~~~~~~~~~~ lib/http1.c:91:3: note: Returning zero, which participates in a condition later 91 | return CURLE_OK; | ^~~~~~~~~~~~~~~ lib/http1.c:107:12: note: Returning from 'detect_line' 107 | result = detect_line(parser, buf, buflen, pnread); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ lib/http1.c:108:7: note: 'result' is 0 108 | if(!result) { | ^~~~~~ lib/http1.c:108:3: note: Taking true branch 108 | if(!result) { | ^ lib/http1.c:109:8: note: Assuming the condition is false 109 | if(curlx_dyn_len(&parser->scratch)) { | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ lib/http1.c:109:5: note: Taking false branch 109 | if(curlx_dyn_len(&parser->scratch)) { | ^ lib/http1.c:118:14: note: Calling 'trim_line' 118 | result = trim_line(parser, options); | ^~~~~~~~~~~~~~~~~~~~~~~~~~ lib/http1.c:55:3: note: Loop condition is false. Exiting loop 55 | DEBUGASSERT(parser->line); | ^ lib/curl_setup.h:1072:24: note: expanded from macro 'DEBUGASSERT' 1072 | #define DEBUGASSERT(x) do {} while(0) | ^ lib/http1.c:56:6: note: Assuming field 'line_len' is 0 56 | if(parser->line_len) { | ^~~~~~~~~~~~~~~~ lib/http1.c:56:3: note: Taking false branch 56 | if(parser->line_len) { | ^ lib/http1.c:68:11: note: Assuming the condition is false 68 | else if(options & H1_PARSE_OPT_STRICT) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ lib/http1.c:68:8: note: Taking false branch 68 | else if(options & H1_PARSE_OPT_STRICT) | ^ lib/http1.c:71:6: note: Assuming field 'line_len' is <= field 'max_line_len' 71 | if(parser->line_len > parser->max_line_len) { | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ lib/http1.c:71:3: note: Taking false branch 71 | if(parser->line_len > parser->max_line_len) { | ^ lib/http1.c:74:3: note: Returning zero, which participates in a condition later 74 | return CURLE_OK; | ^~~~~~~~~~~~~~~ lib/http1.c:118:14: note: Returning from 'trim_line' 118 | result = trim_line(parser, options); | ^~~~~~~~~~~~~~~~~~~~~~~~~~ lib/http1.c:119:8: note: 'result' is 0 119 | if(result) | ^~~~~~ lib/http1.c:119:5: note: Taking false branch 119 | if(result) | ^ lib/http1.c:128:3: note: Returning zero (loaded from 'result'), which participates in a condition later 128 | return result; | ^~~~~~~~~~~~~ lib/http1.c:273:14: note: Returning from 'next_line' 273 | result = next_line(parser, buf, buflen, options, &nread); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ lib/http1.c:274:8: note: 'result' is 0 274 | if(result) { | ^~~~~~ lib/http1.c:274:5: note: Taking false branch 274 | if(result) { | ^ lib/http1.c:282:5: note: Value assigned to 'buf' 282 | buf += nread; | ^~~~~~~~~~~~ lib/http1.c:285:8: note: Assuming field 'line' is null 285 | if(!parser->line) { | ^~~~~~~~~~~~~ lib/http1.c:285:5: note: Taking true branch 285 | if(!parser->line) { | ^ lib/http1.c:287:10: note: Assuming 'buflen' is not equal to 0 287 | if(!buflen) | ^~~~~~~ lib/http1.c:287:7: note: Taking false branch 287 | if(!buflen) | ^ lib/http1.c:272:3: note: Loop condition is true. Entering loop body 272 | while(!parser->done) { | ^ lib/http1.c:273:32: note: Passing null pointer value via 2nd parameter 'buf' 273 | result = next_line(parser, buf, buflen, options, &nread); | ^~~ lib/http1.c:273:14: note: Calling 'next_line' 273 | result = next_line(parser, buf, buflen, options, &nread); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ lib/http1.c:101:14: note: Field 'line' is null 101 | if(parser->line) { | ^ lib/http1.c:101:3: note: Taking false branch 101 | if(parser->line) { | ^ lib/http1.c:107:32: note: Passing null pointer value via 2nd parameter 'buf' 107 | result = detect_line(parser, buf, buflen, pnread); | ^~~ lib/http1.c:107:12: note: Calling 'detect_line' 107 | result = detect_line(parser, buf, buflen, pnread); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ lib/http1.c:83:3: note: Loop condition is false. Exiting loop 83 | DEBUGASSERT(!parser->line); | ^ lib/curl_setup.h:1072:24: note: expanded from macro 'DEBUGASSERT' 1072 | #define DEBUGASSERT(x) do {} while(0) | ^ lib/http1.c:86:6: note: Assuming 'line_end' is non-null 86 | if(!line_end) | ^~~~~~~~~ lib/http1.c:86:3: note: Taking false branch 86 | if(!line_end) | ^ lib/http1.c:88:3: note: Null pointer value stored to field 'line' 88 | parser->line = (const char *)buf; | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ lib/http1.c:89:31: note: Subtraction of a non-null pointer (from variable 'line_end') and a null pointer (via field 'line') results in undefined behavior 89 | parser->line_len = line_end - parser->line + 1; | ~~~~~~~~ ^ ~~~~ 384 warnings generated. ``` Ref: https://github.com/curl/curl/actions/runs/22534731241/job/65279952830?pr=20778#step:11:85 Cherry-picked from curl#20778

augmentcode

Review completed. No suggestions at this time.

Comment augment review to trigger a new review at any time.

v22.1.0 disabled them by default. Fix fallout: - http: check NULL to silence false positives in `HD_VAL()`. Ref: https://releases.llvm.org/22.1.0/tools/clang/tools/extra/docs/ReleaseNotes.html#improvements-to-clang-tidy Follow-up to da6fbb1 #20779 Follow-up to ce4db9c #20751 Closes #20778

github-actions bot added the HTTP label Mar 1, 2026

vszakats mentioned this pull request Mar 1, 2026

clang-tidy: enable clang-analyzer-* checks explicitly, fix fallout #20778

Closed

1 task

bagder approved these changes Mar 2, 2026

View reviewed changes

augmentcode bot reviewed Mar 2, 2026

View reviewed changes

lib/http1.c Show resolved Hide resolved

vszakats added 4 commits March 2, 2026 13:14

add debugassert and return error on NULL ptr

021f960

http1.c set *pnread = 0 before returning error

fa90ebe

cleanup

77ca778

augmentcode bot reviewed Mar 2, 2026

View reviewed changes

vszakats force-pushed the httpnullderef branch from f5465d2 to 77ca778 Compare March 2, 2026 12:15

vszakats closed this in da6fbb1 Mar 2, 2026

vszakats deleted the httpnullderef branch March 2, 2026 12:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

http1: fix potential NULL dereference in `Curl_h1_req_parse_read()`#20779

http1: fix potential NULL dereference in `Curl_h1_req_parse_read()`#20779
vszakats wants to merge 4 commits intocurl:masterfrom
vszakats:httpnullderef

vszakats commented Mar 1, 2026 •

edited

Loading

Uh oh!

vszakats commented Mar 1, 2026

Uh oh!

bagder commented Mar 1, 2026

Uh oh!

bagder commented Mar 2, 2026

Uh oh!

augmentcode bot commented Mar 2, 2026 •

edited

Loading

Uh oh!

augmentcode bot left a comment

Uh oh!

Uh oh!

vszakats commented Mar 2, 2026

Uh oh!

aisle-research-bot bot commented Mar 2, 2026 •

edited

Loading

Uh oh!

augmentcode bot left a comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

vszakats commented Mar 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vszakats commented Mar 1, 2026

Uh oh!

bagder commented Mar 1, 2026

Uh oh!

bagder commented Mar 2, 2026

Uh oh!

augmentcode bot commented Mar 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

augmentcode bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vszakats commented Mar 2, 2026

Uh oh!

aisle-research-bot bot commented Mar 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔒 Aisle Security Analysis

Uh oh!

augmentcode bot left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

vszakats commented Mar 1, 2026 •

edited

Loading

augmentcode bot commented Mar 2, 2026 •

edited

Loading

aisle-research-bot bot commented Mar 2, 2026 •

edited

Loading