Bug #4997
closedFreshnessPeriod may become negative
100%
Description
FreshnessPeriod is defined as a NonNegativeInteger in NDN protocol. ndn-cxx's decoding routines for NNI return a uint64_t
type. In MetaInfo::wireDecode
however, the uint64_t
is converted to time::milliseconds
, which is a signed type and therefore cannot represent the whole range of uint64_t
numbers. This operation has implementation-defined behavior in C++. Typically, large values cause a wraparound and the result becomes negative. Most, if not all, other code in the library and NFD is not prepared to handle a negative FreshnessPeriod, thus causing all sorts of problems. For instance, this NFD assertion will fail.
This bug was found by NFDFuzz, an experimental fuzzer for NFD and ndn-cxx.
Stack trace of the failed assertion in NFD:
fuzzer: ../daemon/fw/forwarder.cpp:526: void nfd::Forwarder::insertDeadNonceList(pit::Entry &, nfd::face::Face *): Assertion `pitEntry.dataFreshnessPeriod >= 0_ms' failed.
==29831== ERROR: libFuzzer: deadly signal
#0 0x602cf7 in __sanitizer_print_stack_trace /tmp/final/llvm.src/projects/compiler-rt/lib/asan/asan_stack.cc:38:3
#1 0x540b46 in fuzzer::Fuzzer::CrashCallback() /tmp/final/llvm.src/projects/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:233:5
#2 0x540b0f in fuzzer::Fuzzer::StaticCrashSignalCallback() /tmp/final/llvm.src/projects/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:206:6
#3 0x7fa90de9538f (/lib/x86_64-linux-gnu/libpthread.so.0+0x1138f)
#4 0x7fa90d18a427 in gsignal (/lib/x86_64-linux-gnu/libc.so.6+0x35427)
#5 0x7fa90d18c029 in abort (/lib/x86_64-linux-gnu/libc.so.6+0x37029)
#6 0x7fa90d182bd6 (/lib/x86_64-linux-gnu/libc.so.6+0x2dbd6)
#7 0x7fa90d182c81 in __assert_fail (/lib/x86_64-linux-gnu/libc.so.6+0x2dc81)
#8 0x814b3a in nfd::Forwarder::insertDeadNonceList(nfd::pit::Entry&, nfd::face::Face*) /users/gjt3/NFD/build/../daemon/fw/forwarder.cpp:526:5
#9 0x81521b in nfd::Forwarder::onIncomingData(nfd::FaceEndpoint const&, ndn::Data const&) /users/gjt3/NFD/build/../daemon/fw/forwarder.cpp:311:11
#10 0x81b0e3 in nfd::Forwarder::Forwarder()::$_2::operator()(nfd::face::Face&) const::{lambda(ndn::Data const&, unsigned long const&)#1}::operator()(ndn::Data const&, unsigned long const&) const /users/gjt3/NFD/build/../daemon/fw/forwarder.cpp:61:15
#11 0x6f28f0 in ndn::util::signal::Signal<nfd::face::LinkService, ndn::Data, unsigned long>::operator()(ndn::Data const&, unsigned long const&) /usr/local/include/ndn-cxx/util/signal/signal.hpp:232:7
#12 0x6f1344 in nfd::face::LinkService::receiveData(ndn::Data const&, unsigned long const&) /users/gjt3/NFD/build/../daemon/face/link-service.cpp:104:3
#13 0x6c2bc8 in nfd::face::GenericLinkService::decodeData(ndn::Block const&, ndn::lp::Packet const&, unsigned long const&) /users/gjt3/NFD/build/../daemon/face/generic-link-service.cpp:445:9
#14 0x6bf908 in nfd::face::GenericLinkService::decodeNetPacket(ndn::Block const&, ndn::lp::Packet const&, unsigned long const&) /users/gjt3/NFD/build/../daemon/face/generic-link-service.cpp:328:15
#15 0x6bedb7 in nfd::face::GenericLinkService::doReceivePacket(ndn::Block const&, unsigned long const&) /users/gjt3/NFD/build/../daemon/face/generic-link-service.cpp:304:13
#16 0x9fe210 in nfd::face::StreamTransport<boost::asio::local::stream_protocol>::handleReceive(boost::system::error_code const&, unsigned long) /users/gjt3/NFD/build/../daemon/face/stream-transport.hpp:258:11
#17 0x9fd9da in _ZN5boost4asio6detail23reactive_socket_recv_opINS0_17mutable_buffers_1EZN3nfd4face15StreamTransportINS0_5local15stream_protocolEE12startReceiveEvEUlDpOT_E_E11do_completeEPNS1_15task_io_serviceEPNS1_25task_io_service_operationERKNS_6system10error_codeEm /usr/include/boost/asio/detail/reactive_socket_recv_op.hpp:110:7
#18 0x67129a in boost::asio::detail::task_io_service::do_run_one(boost::asio::detail::scoped_lock<boost::asio::detail::posix_mutex>&, boost::asio::detail::task_io_service_thread_info&, boost::system::error_code const&) /usr/include/boost/asio/detail/impl/task_io_service.ipp:372:12
#19 0x670a0d in boost::asio::detail::task_io_service::run(boost::system::error_code&) /usr/include/boost/asio/detail/impl/task_io_service.ipp:149:10
#20 0x66e13d in boost::asio::io_service::run() /usr/include/boost/asio/impl/io_service.ipp:59:25
#21 0x66138a in nfd::NfdRunner::run() /users/gjt3/NFD/build/../daemon/fuzzer.cpp:159:15
#22 0x646118 in LLVMFuzzerTestOneInput::$_1::operator()() const /users/gjt3/NFD/build/../daemon/fuzzer.cpp:425:17
#23 0x7fa91540dc7f (/usr/lib/x86_64-linux-gnu/libstdc++.so.6+0xb8c7f)
#24 0x7fa90de8b6b9 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76b9)
Updated by Davide Pesavento over 5 years ago
- Subject changed from Integer overflow in FreshnessPeriod decoding to FreshnessPeriod may become negative
- Description updated (diff)
Updated by Junxiao Shi over 1 year ago
- Assignee set to Junxiao Shi
- Estimated time set to 2.00 h
https://gerrit.named-data.net/c/ndn-cxx/+/7151
FreshnessPeriod cannot go negative in ndn-cxx.
NFD may also need a patch to ensure now+fp is in the future.
Updated by Junxiao Shi over 1 year ago
- Status changed from New to In Progress
- % Done changed from 0 to 40
Updated by Davide Pesavento over 1 year ago
I suppose InterestLifetime suffers from the same issue?
Updated by Junxiao Shi over 1 year ago
- % Done changed from 40 to 50
https://gerrit.named-data.net/c/ndn-cxx/+/7168 InterestLifetime
Updated by Junxiao Shi over 1 year ago
- Status changed from In Progress to Closed
Updated by Davide Pesavento over 1 year ago
- Status changed from Closed to Feedback
Junxiao Shi wrote in #note-4:
NFD may also need a patch to ensure now+fp is in the future.
What about this?
Updated by Davide Pesavento 10 months ago
- Status changed from Feedback to Closed
- % Done changed from 50 to 100
https://gerrit.named-data.net/c/ndn-cxx/+/7375 implements a similar fix for InterestLifetime.
I'm closing this issue since it's about the ndn-cxx bug(s), which should be fixed now. It's unclear to me if there are any related overflow bugs in NFD, but in any case it should be reported as a separate issue.