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.

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

fix(rest): prevent libcurl callback from reading bad address #14615

Merged
merged 1 commit into from
Aug 6, 2024
Merged

fix(rest): prevent libcurl callback from reading bad address #14615

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
19 changes: 18 additions & 1 deletion google/cloud/internal/curl_impl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,12 @@ static std::size_t ReadFunction( // NOLINT(misc-use-anonymous-namespace)
return writev->MoveTo(absl::MakeSpan(buffer, size * nitems));
}

// Instead of trying to send any more bytes from userdata, aborts.
static std::size_t ReadFunctionAbort( // NOLINT(misc-use-anonymous-namespace)
char*, std::size_t, std::size_t, void*) {
return CURL_READFUNC_ABORT;
}

static int SeekFunction( // NOLINT(misc-use-anonymous-namespace)
void* userdata, curl_off_t offset, int origin) {
auto* const writev = reinterpret_cast<WriteVector*>(userdata);
Expand Down Expand Up @@ -518,7 +524,18 @@ Status CurlImpl::MakeRequestImpl(RestContext& context) {
// thus make available the status_code and headers. Any response data
// should be put into the spill buffer, which makes them available for
// subsequent calls to Read() after the headers have been extracted.
return ReadImpl(context, {}).status();
auto read_status = ReadImpl(context, {}).status();

// All data in the WriteVector should have been written by now, unless an
// error, typically a timeout, has occurred. Instruct curl to not attempt to
// send anymore data on this handle. If curl_closed_ is true, then the handle
// has already been recycled and attempting to set an option on it will error.
if (!curl_closed_) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this work in the presence of exceptions? Can exceptions happen at this layer?

status = handle_.SetOption(CURLOPT_READFUNCTION, &ReadFunctionAbort);
if (!status.ok()) return OnTransferError(context, std::move(status));
}

return read_status;
}

StatusOr<std::size_t> CurlImpl::ReadImpl(RestContext& context,
Expand Down
Loading