Skip to content
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.

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

Fixes https://github.com/curl/curl/issues/6696 #6697

Closed
wants to merge 2 commits into from

Conversation

kokke
Copy link
Contributor

@kokke kokke commented Mar 5, 2021

Fixes #6696

From lib/c-hyper.c:445-467 - n is checked against NULL instead of p, at line 467
I have inserted comments at line 445 and 467.

445    p = strchr(n, ':'); /* <-- n assumed non-NULL here, since dereferenced */
446    if(!p)
447      /* this is fine if we already added at least one header */
448      return numh ? CURLE_OK : CURLE_BAD_FUNCTION_ARGUMENT;
449    nlen = p - n;
450    p++; /* move past the colon */
451    while(*p == ' ')
452      p++;
453    v = p;
454    p = strchr(v, '\r');
455    if(!p) {
456      p = strchr(v, '\n');
457      if(p)
458        linelen = 1; /* LF only */
459      else {
460        p = strchr(v, '\0');
461        newline = FALSE; /* no newline */
462      }
463    }
464    else
465      linelen = 2; /* CRLF ending */
466    linelen += (p - n);
467    if(!n) /* <-- n was assumed non-NULL above, should this have been 'if(!p)' ? */
468      return CURLE_BAD_FUNCTION_ARGUMENT;
469    vlen = p - v; 

lib/c-hyper.c Outdated
@@ -464,7 +464,7 @@ CURLcode Curl_hyper_header(struct Curl_easy *data, hyper_headers *headers,
else
linelen = 2; /* CRLF ending */
linelen += (p - n);
if(!n)
if(!p)
Copy link
Member

Choose a reason for hiding this comment

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

AFAICS neither n nor p can be null here so I don't know why that check is there. Do you want to adjust your PR to remove it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You mean remove the entire if-statement?

467    if(!p)
468      return CURLE_BAD_FUNCTION_ARGUMENT;

@jay jay added the Hyper label Mar 5, 2021
@ghost
Copy link

ghost commented Mar 5, 2021

Congratulations 🎉. DeepCode analyzed your code in 2.995 seconds and we found no issues. Enjoy a moment of no bugs ☀️.

👉 View analysis in DeepCode’s Dashboard | Configure the bot

@jay jay added the tidy-up label Mar 5, 2021
@jay jay closed this in 86338ca Mar 5, 2021
@jay
Copy link
Member

jay commented Mar 5, 2021

Thanks

@kokke
Copy link
Contributor Author

kokke commented Mar 5, 2021

Thank you for super-swift action @jay 👍 have a great weekend :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

possibly null-check against wrong variable
2 participants