CVE-2019-11934 (C++, published 2019-12-04T17:16Z)
Description: value: Improper handling of close_notify alerts can result in an out-of-bounds read in AsyncSSLSocket. This issue affects folly prior to v2019.11.04.00.
Severity: HIGH (CVSS v3 base score: 9.8)
Weakness: CWE-125 \u2014 Out-of-bounds Read
The product reads data past the end, or before the beginning, of the intended buffer.
Repository: https://github.com/facebook/folly
Files changed:
folly/io/async/AsyncSSLSocket.cpp
folly/io/async/test/AsyncSSLSocketTest.cpp
folly/io/async/test/AsyncSSLSocketTest.h
Commit message: Handle close_notify as standard writeErr in AsyncSSLSocket.

Summary: Fixes CVE-2019-11934

Reviewed By: mingtaoy

Differential Revision: D18020613

fbshipit-source-id: db82bb250e53f0d225f1280bd67bc74abd417836


Vulnerable code:
    if (zero_return(error, rc, errno)) {
      return WriteResult(0);
    }
      auto writeResult = interpretSSLError(int(bytes), error);
      if (writeResult.writeReturn < 0) {
        return writeResult;
      } // else fall through to below to correctly record totalWritten
    wcb_->setSocket(socket_);

Fixed code:
      return interpretSSLError(int(bytes), error);
class PerLoopReadCallback : public AsyncTransportWrapper::ReadCallback {
 public:
  void getReadBuffer(void** bufReturn, size_t* lenReturn) override {
    *bufReturn = buf_.data();
    *lenReturn = buf_.size();
  }

  void readDataAvailable(size_t len) noexcept override {
    VLOG(3) << "Read of size: " << len;
    s_->setReadCB(nullptr);
    s_->getEventBase()->runInLoop([this]() { s_->setReadCB(this); });
  }

  void readErr(const AsyncSocketException&) noexcept override {}

  void readEOF() noexcept override {}

  void setSocket(AsyncSocket* s) {
    s_ = s;
  }

 private:
  AsyncSocket* s_;
  std::array<uint8_t, 1000> buf_;
};

class CloseNotifyConnector : public AsyncSocket::ConnectCallback {
 public:
  CloseNotifyConnector(EventBase* evb, const SocketAddress& addr) {
    evb_ = evb;
    ssl_ = AsyncSSLSocket::newSocket(std::make_shared<SSLContext>(), evb_);
    ssl_->connect(this, addr);
  }

  void connectSuccess() noexcept override {
    ssl_->writeChain(nullptr, IOBuf::copyBuffer("hi"));
    auto ssl = const_cast<SSL*>(ssl_->getSSL());
    SSL_shutdown(ssl);
    auto fd = ssl_->detachNetworkSocket();
    tcp_.reset(new AsyncSocket(evb_, fd), AsyncSocket::Destructor());
    evb_->runAfterDelay(
        [this]() {
          perLoopReads_.setSocket(tcp_.get());
          tcp_->setReadCB(&perLoopReads_);
          evb_->runAfterDelay([this]() { tcp_->closeNow(); }, 10);
        },
        100);
  }

  void connectErr(const AsyncSocketException& ex) noexcept override {
    FAIL() << ex.what();
  }

 private:
  EventBase* evb_;
  std::shared_ptr<AsyncSSLSocket> ssl_;
  std::shared_ptr<AsyncSocket> tcp_;
  PerLoopReadCallback perLoopReads_;
};

class ErrorCheckingWriteCallback : public AsyncSocket::WriteCallback {
 public:
  void writeSuccess() noexcept override {}

  void writeErr(size_t, const AsyncSocketException& ex) noexcept override {
    LOG(ERROR) << "write error: " << ex.what();
    EXPECT_NE(
        ex.getType(),
        AsyncSocketException::AsyncSocketExceptionType::SSL_ERROR);
  }
};

class WriteOnEofReadCallback : public ReadCallback {
 public:
  using ReadCallback::ReadCallback;

  void readEOF() noexcept override {
    LOG(INFO) << "Got EOF";
    auto chain = IOBuf::create(0);
    for (size_t i = 0; i < 1000 * 1000; i++) {
      auto buf = IOBuf::create(10);
      buf->append(10);
      memset(buf->writableData(), 'x', 10);
      chain->prependChain(std::move(buf));
    }
    socket_->writeChain(&writeCallback_, std::move(chain));
  }

  void readErr(const AsyncSocketException& ex) noexcept override {
    LOG(ERROR) << ex.what();
  }

 private:
  ErrorCheckingWriteCallback writeCallback_;
};

TEST(AsyncSSLSocketTest, EarlyCloseNotify) {
  WriteOnEofReadCallback readCallback(nullptr);
  HandshakeCallback handshakeCallback(&readCallback);
  SSLServerAcceptCallback acceptCallback(&handshakeCallback);
  TestSSLServer server(&acceptCallback);

  EventBase eventBase;
  CloseNotifyConnector cnc(&eventBase, server.getAddress());

  eventBase.loop();
}

    if (wcb_) {
      wcb_->setSocket(socket_);
    }