Skip to content

SPTCH-3194: fix fail code returned if 401 Unauthorized on range request#3

Merged
sergio-nsk merged 1 commit intosnxd-7_84_0from
sergey/SPTCH-3194/3
Sep 1, 2022
Merged

SPTCH-3194: fix fail code returned if 401 Unauthorized on range request#3
sergio-nsk merged 1 commit intosnxd-7_84_0from
sergey/SPTCH-3194/3

Conversation

@sergio-nsk
Copy link
Copy Markdown

Data range can be requested and 401 Unauthorized can be responded with a body
unrelated to the requested content, and without Content-Range header, with
unrelated Content-Length value. curl_easy_perform() fails with the error
CURLE_RANGE_ERROR. It should not fail.

@sergio-nsk sergio-nsk added the bug Something isn't working label Aug 31, 2022
@sergio-nsk sergio-nsk requested a review from nmoinvaz August 31, 2022 23:24
Comment thread lib/http.c Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why have the ! symbol when you can rewrite the expression? Is that syntax common in curl?

Copy link
Copy Markdown
Author

@sergio-nsk sergio-nsk Sep 1, 2022

Choose a reason for hiding this comment

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

Are you going to review curl code? Let their vendors do it. If they accept the fix, the code above goes away.

Copy link
Copy Markdown
Author

@sergio-nsk sergio-nsk Sep 1, 2022

Choose a reason for hiding this comment

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

It's common in curl code - negate what is expected for early exit:

  if(!(servers && servers[0]))
  
  if(!(ac && t))
  
    if(!(((pCertContextServer->dwCertEncodingType & X509_ASN_ENCODING) != 0) &&
         (pCertContextServer->cbCertEncoded > 0)))

Data range can be requested and 401 Unauthorized can be responded with a body
unrelated to the requested content, and without Content-Range header, with
unrelated Content-Length value. `curl_easy_perform()` fails with the error
`CURLE_RANGE_ERROR`. It should not fail.
@sergio-nsk
Copy link
Copy Markdown
Author

The commit message is changed here as the vendor changed it there.

@nmoinvaz
Copy link
Copy Markdown

nmoinvaz commented Sep 1, 2022

Linked PR: curl#9401.

Copy link
Copy Markdown

@nmoinvaz nmoinvaz left a comment

Choose a reason for hiding this comment

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

If vendor requests changes we should update the repo.

@sergio-nsk sergio-nsk merged commit 435e526 into snxd-7_84_0 Sep 1, 2022
@sergio-nsk sergio-nsk deleted the sergey/SPTCH-3194/3 branch September 1, 2022 23:02
nmoinvaz pushed a commit that referenced this pull request Jul 1, 2023
Fixes msan warnings:

==54195==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x55ece35e57cb in line_length /home/runner/work/curl/curl/tests/server/getpart.c:111:25
    #1 0x55ece35e3b83 in readline /home/runner/work/curl/curl/tests/server/getpart.c:164:24
    #2 0x55ece35e0269 in getpart /home/runner/work/curl/curl/tests/server/getpart.c:347:18
    #3 0x55ece36180b6 in parse_servercmd /home/runner/work/curl/curl/tests/server/sws.c:283:13

Closes curl#10822
sergio-nsk added a commit that referenced this pull request Jul 31, 2023
Further `u->path = Curl_memdup(path, pathlen + 1);` accesses bytes after the null-terminator.
```
==2676==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x04d48c75 at pc 0x0112708a bp 0x006fb7e0 sp 0x006fb3c4
READ of size 78 at 0x04d48c75 thread T0
    #0 0x1127089 in __asan_wrap_memcpy D:\a\_work\1\s\src\vctools\asan\llvm\compiler-rt\lib\sanitizer_common\sanitizer_common_interceptors.inc:840
    #1 0x1891a0e in Curl_memdup C:\actions-runner\_work\client\client\third_party\curl\lib\strdup.c:97
    #2 0x18db4b0 in parseurl C:\actions-runner\_work\client\client\third_party\curl\lib\urlapi.c:1297
    #3 0x18db819 in parseurl_and_replace C:\actions-runner\_work\client\client\third_party\curl\lib\urlapi.c:1342
    #4 0x18d6e39 in curl_url_set C:\actions-runner\_work\client\client\third_party\curl\lib\urlapi.c:1790
    #5 0x1877d3e in parseurlandfillconn C:\actions-runner\_work\client\client\third_party\curl\lib\url.c:1768
    #6 0x1871acf in create_conn C:\actions-runner\_work\client\client\third_party\curl\lib\url.c:3403
    curl#7 0x186d8dc in Curl_connect C:\actions-runner\_work\client\client\third_party\curl\lib\url.c:3888
    curl#8 0x1856b78 in multi_runsingle C:\actions-runner\_work\client\client\third_party\curl\lib\multi.c:1982
    curl#9 0x18531e3 in curl_multi_perform C:\actions-runner\_work\client\client\third_party\curl\lib\multi.c:2756
```
nmoinvaz pushed a commit that referenced this pull request Aug 8, 2023
`u->path = Curl_memdup(path, pathlen + 1);` accesses bytes after the null-terminator.

```
==2676==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x04d48c75 at pc 0x0112708a bp 0x006fb7e0 sp 0x006fb3c4
READ of size 78 at 0x04d48c75 thread T0
    #0 0x1127089 in __asan_wrap_memcpy D:\a\_work\1\s\src\vctools\asan\llvm\compiler-rt\lib\sanitizer_common\sanitizer_common_interceptors.inc:840
    #1 0x1891a0e in Curl_memdup C:\actions-runner\_work\client\client\third_party\curl\lib\strdup.c:97
    #2 0x18db4b0 in parseurl C:\actions-runner\_work\client\client\third_party\curl\lib\urlapi.c:1297
    #3 0x18db819 in parseurl_and_replace C:\actions-runner\_work\client\client\third_party\curl\lib\urlapi.c:1342
    #4 0x18d6e39 in curl_url_set C:\actions-runner\_work\client\client\third_party\curl\lib\urlapi.c:1790
    #5 0x1877d3e in parseurlandfillconn C:\actions-runner\_work\client\client\third_party\curl\lib\url.c:1768
    #6 0x1871acf in create_conn C:\actions-runner\_work\client\client\third_party\curl\lib\url.c:3403
    curl#7 0x186d8dc in Curl_connect C:\actions-runner\_work\client\client\third_party\curl\lib\url.c:3888
    curl#8 0x1856b78 in multi_runsingle C:\actions-runner\_work\client\client\third_party\curl\lib\multi.c:1982
    curl#9 0x18531e3 in curl_multi_perform C:\actions-runner\_work\client\client\third_party\curl\lib\multi.c:2756
```

Closes curl#11560
sergio-nsk pushed a commit that referenced this pull request Apr 21, 2024
In order to make MSAN happy:

    ==2200945==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x596f3b3ed246 in curlx_strtoofft [...]/libcurl/src/lib/strtoofft.c:239:11
    #1 0x596f3b402156 in Curl_httpchunk_read [...]/libcurl/src/lib/http_chunks.c:149:12
    #2 0x596f3b348550 in readwrite_data [...]/libcurl/src/lib/transfer.c:607:11
    [...]

    ==2202041==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x5a3fab66a72a in Curl_parse_port [...]/libcurl/src/lib/urlapi.c:547:8
    #1 0x5a3fab650645 in parse_authority [...]/libcurl/src/lib/urlapi.c:796:12
    #2 0x5a3fab6740f6 in parseurl [...]/libcurl/src/lib/urlapi.c:1176:16
    #3 0x5a3fab664fc5 in parseurl_and_replace [...]/libcurl/src/lib/urlapi.c:1342:12
    [...]

    ==2202320==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x569076a0d6b0 in ipv4_normalize [...]/libcurl/src/lib/urlapi.c:683:12
    #1 0x5690769f2820 in parse_authority [...]/libcurl/src/lib/urlapi.c:803:10
    #2 0x569076a160f6 in parseurl [...]/libcurl/src/lib/urlapi.c:1176:16
    #3 0x569076a06fc5 in parseurl_and_replace [...]/libcurl/src/lib/urlapi.c:1342:12
    [...]

Signed-off-by: Louis Solofrizzo <lsolofrizzo@scaleway.com>
Closes curl#12995
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants