diff --git a/services/network/public/cpp/source_stream_to_data_pipe.cc b/services/network/public/cpp/source_stream_to_data_pipe.cc index d6ade7b0ec52de7cda9351c4dd84c173c94defe1..615804ad8d293eaa44a395547eb78ec3fd3d9b88 100644 --- a/services/network/public/cpp/source_stream_to_data_pipe.cc +++ b/services/network/public/cpp/source_stream_to_data_pipe.cc @@ -53,9 +53,9 @@ void SourceStreamToDataPipe::ReadMore() { scoped_refptr buffer( new network::NetToMojoIOBuffer(pending_write_.get())); - int result = source_->Read( - buffer.get(), base::checked_cast(num_bytes), - base::BindOnce(&SourceStreamToDataPipe::DidRead, base::Unretained(this))); + int result = source_->Read(buffer.get(), base::checked_cast(num_bytes), + base::BindOnce(&SourceStreamToDataPipe::DidRead, + weak_factory_.GetWeakPtr())); if (result != net::ERR_IO_PENDING) DidRead(result); diff --git a/services/network/public/cpp/source_stream_to_data_pipe_unittest.cc b/services/network/public/cpp/source_stream_to_data_pipe_unittest.cc index 07ef0bf063b50a96b2bd57362241d16c2809563f..417a574019aa99af1af4e0f839157a2605c2d822 100644 --- a/services/network/public/cpp/source_stream_to_data_pipe_unittest.cc +++ b/services/network/public/cpp/source_stream_to_data_pipe_unittest.cc @@ -6,7 +6,9 @@ #include "base/bind.h" #include "base/memory/raw_ptr.h" +#include "base/test/bind.h" #include "base/test/task_environment.h" +#include "net/base/net_errors.h" #include "net/filter/mock_source_stream.h" #include "testing/gtest/include/gtest/gtest.h" #include "third_party/abseil-cpp/absl/types/optional.h" @@ -42,6 +44,33 @@ struct SourceStreamToDataPipeTestParam { const ReadResultType read_result_type; }; +class DummyPendingSourceStream : public net::SourceStream { + public: + DummyPendingSourceStream() : net::SourceStream(SourceStream::TYPE_NONE) {} + ~DummyPendingSourceStream() override = default; + + DummyPendingSourceStream(const DummyPendingSourceStream&) = delete; + DummyPendingSourceStream& operator=(const DummyPendingSourceStream&) = delete; + + // SourceStream implementation + int Read(net::IOBuffer* dest_buffer, + int buffer_size, + net::CompletionOnceCallback callback) override { + callback_ = std::move(callback); + return net::ERR_IO_PENDING; + } + std::string Description() const override { return ""; } + bool MayHaveMoreBytes() const override { return true; } + + net::CompletionOnceCallback TakeCompletionCallback() { + CHECK(callback_); + return std::move(callback_); + } + + private: + net::CompletionOnceCallback callback_; +}; + } // namespace class SourceStreamToDataPipeTest @@ -212,4 +241,33 @@ TEST_P(SourceStreamToDataPipeTest, MayHaveMoreBytes) { EXPECT_EQ(ReadPipe(&output), net::OK); EXPECT_EQ(output, message); } + +TEST(SourceStreamToDataPipeCallbackTest, CompletionCallbackAfterDestructed) { + base::test::TaskEnvironment task_environment; + + std::unique_ptr source = + std::make_unique(); + DummyPendingSourceStream* source_ptr = source.get(); + const MojoCreateDataPipeOptions data_pipe_options{ + sizeof(MojoCreateDataPipeOptions), MOJO_CREATE_DATA_PIPE_FLAG_NONE, 1, 1}; + mojo::ScopedDataPipeProducerHandle producer_end; + mojo::ScopedDataPipeConsumerHandle consumer_end; + CHECK_EQ(MOJO_RESULT_OK, mojo::CreateDataPipe(&data_pipe_options, + producer_end, consumer_end)); + + std::unique_ptr adapter = + std::make_unique(std::move(source), + std::move(producer_end)); + bool callback_called = false; + adapter->Start( + base::BindLambdaForTesting([&](int result) { callback_called = true; })); + net::CompletionOnceCallback callback = source_ptr->TakeCompletionCallback(); + adapter.reset(); + + // Test that calling `callback` after deleting `adapter` must not cause UAF + // (crbug.com/1511085). + std::move(callback).Run(net::ERR_FAILED); + EXPECT_FALSE(callback_called); +} + } // namespace network