Skip to content

Commit

Permalink
fix(rest): prevent libcurl callback from reading bad address (#14626)
Browse files Browse the repository at this point in the history
  • Loading branch information
dbolduc authored Aug 9, 2024
1 parent aaf88be commit 567d8f2
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 0 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@
breaking changes in the upcoming 3.x release. This release is scheduled for
2024-12 or 2025-01.

## v2.25.1 - 2024-08

- fix(rest): prevent libcurl callback from reading bad address ([#14615](https://github.com/googleapis/google-cloud-cpp/pull/14615), [#14617](https://github.com/googleapis/google-cloud-cpp/pull/14617))

## v2.25.0 - 2024-06

### New Libraries
Expand Down
27 changes: 27 additions & 0 deletions google/cloud/internal/curl_impl.cc
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 @@ -482,6 +488,22 @@ std::size_t CurlImpl::HeaderCallback(absl::Span<char> response) {
response.size());
}

class CurlImpl::ReadFunctionAbortGuard {
public:
explicit ReadFunctionAbortGuard(CurlImpl& impl) : impl_(impl) {}
~ReadFunctionAbortGuard() {
// If curl_closed_ is true, then the handle has already been recycled and
// attempting to set an option on it will error.
if (!impl_.curl_closed_) {
impl_.handle_.SetOptionUnchecked(CURLOPT_READFUNCTION,
&ReadFunctionAbort);
}
}

private:
CurlImpl& impl_;
};

Status CurlImpl::MakeRequestImpl(RestContext& context) {
TRACE_STATE() << ", url_=" << url_;

Expand All @@ -504,6 +526,11 @@ Status CurlImpl::MakeRequestImpl(RestContext& context) {
handle_.SetOptionUnchecked(CURLOPT_HTTP_VERSION,
VersionToCurlCode(http_version_));

// All data in the WriteVector should be written after ReadImpl returns unless
// an error, typically a timeout, has occurred. Use ReadFunctionAbortGuard to
// leverage RAII to instruct curl to not attempt to send anymore data on this
// handle regardless if an error or exception is encountered.
ReadFunctionAbortGuard guard(*this);
auto error = curl_multi_add_handle(multi_.get(), handle_.handle_.get());

// This indicates that we are using the API incorrectly. The application
Expand Down
1 change: 1 addition & 0 deletions google/cloud/internal/curl_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ class CurlImpl {
std::size_t HeaderCallback(absl::Span<char> response);

private:
class ReadFunctionAbortGuard;
Status MakeRequestImpl(RestContext& context);
StatusOr<std::size_t> ReadImpl(RestContext& context, absl::Span<char> output);

Expand Down

0 comments on commit 567d8f2

Please sign in to comment.