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

Jump to bottom

MacOS (Arm64) crash on curl += 7.76 #7308

Closed
richardmarion opened this issue Jun 28, 2021 · 7 comments
Closed

MacOS (Arm64) crash on curl += 7.76 #7308

richardmarion opened this issue Jun 28, 2021 · 7 comments
Labels
crash HTTP

Comments

@richardmarion
Copy link

With the addition of the statement of "full-size = (size_t) data->set.max_send_speed" on line 1179-1182 in http.c in commit 24e469f#diff-14c8235d370910f1781de1a00ed1957ec948e84e88fdc52db87bd4eb7c7167ff made for 7.76, MacOS crashes in memcpy on line 1205.

I did this. Built version 7.75 and version 7.76 from sources for arm64 target. Once I took out line 1179-1182 from http.c, I was able to run overnight without memory violation crashes.

I expected the following. I expected both versions to work

curl/libcurl version - 7.75 & 7.76

[curl -V output]

MacOS 11.4
@bagder
Copy link
Member

bagder commented Jun 28, 2021

But why does it crash there? Isn't fullsize smaller than http->postsize in that case so that it copies less data than it would otherwise?

@richardmarion
Copy link
Author

richardmarion commented Jun 28, 2021 via email

@bagder
Copy link
Member

bagder commented Jun 28, 2021

I can also send .crashes if it helps

Nah, I wouldn't know what to do with it.

@jay
Copy link
Member

jay commented Jun 29, 2021

curl/lib/http.c

Lines 1163 to 1170 in 6b951a6

static size_t readmoredata(char *buffer,
size_t size,
size_t nitems,
void *userp)
{
struct Curl_easy *data = (struct Curl_easy *)userp;
struct HTTP *http = data->req.p.http;
size_t fullsize = size * nitems;

curl/lib/http.c

Lines 1179 to 1182 in 6b951a6

if(data->set.max_send_speed &&
(data->set.max_send_speed < http->postsize))
/* speed limit */
fullsize = (size_t)data->set.max_send_speed;

curl/lib/http.c

Line 1205 in 6b951a6

memcpy(buffer, http->postdata, fullsize);

fullsize should not be larger than the passed in fullsize which is the size of buffer. My guess is the max speed that is assigned to fullsize is larger than the passed in fullsize.

diff --git a/lib/http.c b/lib/http.c
index 6d5d8fb..7b44198 100644
--- a/lib/http.c
+++ b/lib/http.c
@@ -1177,6 +1177,7 @@ static size_t readmoredata(char *buffer,
   data->req.forbidchunk = (http->sending == HTTPSEND_REQUEST)?TRUE:FALSE;
 
   if(data->set.max_send_speed &&
+     (data->set.max_send_speed < fullsize) &&
      (data->set.max_send_speed < http->postsize))
     /* speed limit */
     fullsize = (size_t)data->set.max_send_speed;

@bagder

This comment has been minimized.

Sign in to view
bagder added a commit that referenced this issue Jun 29, 2021
@bagder
http:readmoredata don't copy more data than what fits in buffer
948ab70 
Fixed-by: Jay Satiro
Reported-by: Richard Marion
Fixes #7308
@bagder bagder mentioned this issue Jun 29, 2021
Closed
@bagder
Copy link
Member

bagder commented Jun 29, 2021

I decided it would be quicker if I just made a PR out of @jay's proposal. See #7315

jay added a commit that referenced this issue Jun 29, 2021
@jay
http: fix crash in upload
d8c9d77 
- Don't set the size of the piece of data to send to the rate limit if
  that limit is larger than the buffer size that will hold the piece.

Prior to this change if CURLOPT_MAX_SEND_SPEED_LARGE
(curl tool: --limit-rate) was set then it was possible that a temporary
buffer used for uploading could be written to out of bounds. A likely
scenario for this would be a non-trivial amount of post data combined
with a rate limit larger than CURLOPT_UPLOAD_BUFFERSIZE (default 64k).

The bug was introduced in 24e469f which precedes 7.77.0.

perl -e "print '0' x 200000" > tmp
curl --limit-rate 128k -d @tmp httpbin.org/post

Reported-by: Richard Marion

Fixes #7308
Closes #xxxx
@jay
Copy link
Member

jay commented Jun 29, 2021

I was working on the commit message at the same time you opened so I force pushed to your branch

jay added a commit that referenced this issue Jun 29, 2021
@jay
http: fix crash in upload
7174c66 
- Don't set the size of the piece of data to send to the rate limit if
  that limit is larger than the buffer size that will hold the piece.

Prior to this change if CURLOPT_MAX_SEND_SPEED_LARGE
(curl tool: --limit-rate) was set then it was possible that a temporary
buffer used for uploading could be written to out of bounds. A likely
scenario for this would be a non-trivial amount of post data combined
with a rate limit larger than CURLOPT_UPLOAD_BUFFERSIZE (default 64k).

The bug was introduced in 24e469f which is in releases since 7.76.0.

perl -e "print '0' x 200000" > tmp
curl --limit-rate 128k -d @tmp httpbin.org/post

Reported-by: Richard Marion

Fixes #7308
Closes #xxxx
@jay jay closed this as completed in ca88934 Jun 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
crash HTTP
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

http:readmoredata don't copy more data than what fits in buffer curl/curl
3 participants
@bagder @jay @richardmarion