Skip to content

Commit

Permalink
fix invalid-enum-load UBSAN error in SPDYConstants.cpp
Browse files Browse the repository at this point in the history
Summary:
Exposed by UBSAN:
```lang=bash
  proxygen/lib/http/codec/SPDYConstants.cpp:67:11: runtime error: load of value 314159, which is not a valid value for type 'proxygen::spdy::ResetStatusCode'
      #0 0x7f81bb4b39d9 in proxygen::spdy::rstToErrorCode(proxygen::spdy::ResetStatusCode) proxygen/lib/http/codec/SPDYConstants.cpp:67
      #1 0x7f81bb443d31 in proxygen::SPDYCodec::onRstStream(unsigned int) proxygen/lib/http/codec/SPDYCodec.cpp:1331
      #2 0x7f81bb43f559 in proxygen::SPDYCodec::onControlFrame(folly::io::Cursor&) proxygen/lib/http/codec/SPDYCodec.cpp:376
      #3 0x7f81bb43c305 in proxygen::SPDYCodec::parseIngress(folly::IOBuf const&) proxygen/lib/http/codec/SPDYCodec.cpp:309
      #4 0x7f81bb43a4ec in proxygen::SPDYCodec::onIngress(folly::IOBuf const&) proxygen/lib/http/codec/SPDYCodec.cpp:243
      #5 0x7f81cdf12854 in proxygen::PassThroughHTTPCodecFilter::onIngress(folly::IOBuf const&) proxygen/lib/http/codec/HTTPCodecFilter.cpp:170
      #6 0x7f81cdf12854 in proxygen::PassThroughHTTPCodecFilter::onIngress(folly::IOBuf const&) proxygen/lib/http/codec/HTTPCodecFilter.cpp:170
      #7 0x7f81cdf12854 in proxygen::PassThroughHTTPCodecFilter::onIngress(folly::IOBuf const&) proxygen/lib/http/codec/HTTPCodecFilter.cpp:170
      #8 0x7f81cdf12854 in proxygen::PassThroughHTTPCodecFilter::onIngress(folly::IOBuf const&) proxygen/lib/http/codec/HTTPCodecFilter.cpp:170
      #9 0x7f81d0da0239 in proxygen::HTTPSession::processReadData() proxygen/lib/http/session/HTTPSession.cpp:487
      #10 0x7f81d0d9fe39 in proxygen::HTTPSession::readDataAvailable(unsigned long) proxygen/lib/http/session/HTTPSession.cpp:440
      #11 0x7f81e6daea85 in folly::AsyncSocket::handleRead() folly/io/async/AsyncSocket.cpp:1900
      #12 0x7f81e6da93f9 in folly::AsyncSocket::ioReady(unsigned short) folly/io/async/AsyncSocket.cpp:1694
      #13 0x7f81e6dc14ee in folly::AsyncSocket::IoHandler::handlerReady(unsigned short) folly/io/async/AsyncSocket.h:976
      #14 0x7f81e6e5e91b in folly::EventHandler::libeventCallback(int, short, void*) folly/io/async/EventHandler.cpp:160
      #15 0x7f81de739b11 in event_process_active /home/engshare/third-party2/libevent/1.4.14b_hphp/src/libevent-1.4.14b-stable/event.c:390
      #16 0x7f81de739b11 in event_base_loop /home/engshare/third-party2/libevent/1.4.14b_hphp/src/libevent-1.4.14b-stable/event.c:532
      #17 0x7f81e6e11864 in folly::EventBase::loopBody(int) folly/io/async/EventBase.cpp:313
      #18 0x7f81e6e10726 in folly::EventBase::loop() folly/io/async/EventBase.cpp:252
      #19 0x7f81e6e14e85 in folly::EventBase::loopForever() folly/io/async/EventBase.cpp:450
      #20 0x7f81c4141a7b in proxygen::WorkerThread::runLoop() proxygen/lib/services/WorkerThread.cpp:137
      #21 0x7f81c4143afe in proxygen::WorkerThread::start()::$_0::operator()() proxygen/lib/services/WorkerThread.cpp:41
      #22 0x7f81c4143a34 in void std::_Bind_simple<proxygen::WorkerThread::start()::$_0 ()>::_M_invoke<>(std::_Index_tuple<>) third-party-buck/gcc-5-glibc-2.23/build/libgcc/include/c++/trunk/functional:1530
      #23 0x7f81c4143a04 in std::_Bind_simple<proxygen::WorkerThread::start()::$_0 ()>::operator()() third-party-buck/gcc-5-glibc-2.23/build/libgcc/include/c++/trunk/functional:1520
      #24 0x7f81c41438d8 in std::thread::_Impl<std::_Bind_simple<proxygen::WorkerThread::start()::$_0 ()> >::_M_run() third-party-buck/gcc-5-glibc-2.23/build/libgcc/include/c++/trunk/thread:115
      #25 0x7f81da7ef170 in execute_native_thread_routine /home/engshare/third-party2/libgcc/5.x/src/gcc-5/x86_64-facebook-linux/libstdc++-v3/src/c++11/../../../.././libstdc++-v3/src/c++11/thread.cc:84
      #26 0x7f81d7e1f7a8 in start_thread /home/engshare/third-party2/glibc/2.23/src/glibc-2.23/nptl/pthread_create.c:333
      #27 0x7f81d71f9a7c in __clone /home/engshare/third-party2/glibc/2.23/src/glibc-2.23/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:109

  SUMMARY: UndefinedBehaviorSanitizer: invalid-enum-load proxygen/lib/http/codec/SPDYConstants.cpp:67:11
```
Here we have a typical function that converts from one enum type to another. There is a special validation logic in place, if the original enum value is not among predetermined values the result is a "special" error value. UBSAN detected that the argumemnt value which was provided was invalid for the original enum type. To prevent the UB and potential overflow use an int type.

Reviewed By: w-o-o

Differential Revision: D7185566

fbshipit-source-id: 613c8c6aa64d24c48889810e56a79fd370cdb32b
  • Loading branch information
Igor Sugak authored and facebook-github-bot committed Mar 8, 2018
1 parent 8180833 commit 0b74038
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 2 deletions.
2 changes: 1 addition & 1 deletion proxygen/lib/http/codec/SPDYConstants.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ ErrorCode goawayToErrorCode(GoawayStatusCode code) {
return ErrorCode::PROTOCOL_ERROR;
}

ErrorCode rstToErrorCode(ResetStatusCode code) {
ErrorCode rstToErrorCode(uint32_t code) {
switch (code) {
case RST_PROTOCOL_ERROR: break;
case RST_INVALID_STREAM: return ErrorCode::_SPDY_INVALID_STREAM;
Expand Down
2 changes: 1 addition & 1 deletion proxygen/lib/http/codec/SPDYConstants.h
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ enum GoawayStatusCode {
extern GoawayStatusCode errorCodeToGoaway(ErrorCode code);
extern ResetStatusCode errorCodeToReset(ErrorCode code);
extern ErrorCode goawayToErrorCode(spdy::GoawayStatusCode);
extern ErrorCode rstToErrorCode(spdy::ResetStatusCode);
extern ErrorCode rstToErrorCode(uint32_t);

folly::Optional<proxygen::spdy::SettingsId> httpToSpdySettingsId(
proxygen::SettingsId id);
Expand Down

0 comments on commit 0b74038

Please sign in to comment.