Bug #4726
closed
Block class mishandles type == 0xffffffff (was: NFD crashes on unexpected input)
100%
Description
STEPS TO REPRODUCE:
- Start an instance of NFD
- run the command
echo -e '\xfe\xff\xff\xff\xff\x00' | nc 127.0.0.1 6363
EXPECTED:
- NFD does not crash
OBSERVED:
- NFD crashes
Updated by price clark over 5 years ago
https://github.com/peetonn/ndn-docker/tree/master/node
was using this build so this happened using the bleeding edge NFD/ndn-cxx.
Updated by Davide Pesavento over 5 years ago
- Start date deleted (
08/30/2018)
Can you provide a gdb backtrace obtained with "bt full"?
Updated by Davide Pesavento over 5 years ago
- run the command `nc -z -v
This command is incomplete.
Updated by Davide Pesavento over 5 years ago
- Status changed from New to Rejected
I've tried to reproduce using various combinations of nc options, and NFD never crashed. Critical info is missing from the bug report, so it's impossible to debug further. Rejecting.
Updated by price clark about 5 years ago
echo -e '\xfe\xff\xff\xff\xff\x00' | nc 127.0.0.1 6363
courtesy of Sam McKay
Updated by Junxiao Shi about 5 years ago
- Description updated (diff)
- Status changed from Rejected to New
I'm able to reproduce this bug in NFD 0.6.4.
Crash happens on UDP as well.
Updated by Davide Pesavento about 5 years ago
With git master branch:
1553024644.794751 TRACE: [nfd.TcpChannel] [tcp4://0.0.0.0:6363] Incoming connection from 127.0.0.1:46642
1553024644.794877 INFO: [nfd.TcpTransport] [id=0,local=tcp4://127.0.0.1:6363,remote=tcp4://127.0.0.1:46642] Creating transport
1553024644.794906 INFO: [nfd.FaceTable] Added face id=261 remote=tcp4://127.0.0.1:46642 local=tcp4://127.0.0.1:6363
1553024644.795208 TRACE: [nfd.TcpTransport] [id=261,local=tcp4://127.0.0.1:6363,remote=tcp4://127.0.0.1:46642] Received: 7 bytes
1553024644.795278 FATAL: [nfd.Main] ../ndn-cxx/encoding/block.cpp(292): Throw in function size
Dynamic exception type: boost::exception_detail::clone_impl<boost::exception_detail::error_info_injector<ndn::Block::Error> >
std::exception::what: Block size cannot be determined (undefined block size)
===== Stacktrace =====
0# ndn::Block::size() const at ../ndn-cxx/encoding/block.cpp:292
1# nfd::face::StreamTransport<boost::asio::ip::tcp>::handleReceive(boost::system::error_code const&, unsigned long) at ../daemon/face/stream-transport.hpp:255
2# _ZN5boost4asio6detail23reactive_socket_recv_opINS0_17mutable_buffers_1EZN3nfd4face15StreamTransportINS0_2ip3tcpEE12startReceiveEvEUlDpOT_E_E11do_completeEPvPNS1_19scheduler_operationERKNS_6system10error_codeEm at /usr/include/boost/asio/detail/reactive_socket_recv_op.hpp:122
3# boost::asio::detail::scheduler::run(boost::system::error_code&) at /usr/include/boost/asio/detail/impl/scheduler.ipp:154
4# nfd::NfdRunner::run() at ../daemon/main.cpp:155
5# main at ../daemon/main.cpp:335
6# 0x00007F3D450BA09B in /lib/x86_64-linux-gnu/libc.so.6
7# 0x000055ED4AF08D0A in ./build/bin/nfd
======================
Updated by Davide Pesavento about 5 years ago
- Project changed from NFD to ndn-cxx
- Subject changed from NFD crashes when it gets unexpected input to NFD crashes on unexpected input
- Category set to Base
price clark wrote:
echo -e '\xfe\xff\xff\xff\xff\x00' | nc 127.0.0.1 6363
Parsed as TLV, the above blob has a type == 0xffffffff, i.e. the max value for a uint32_t. The ndn::Block class uses this value as sentinel to indicate an invalid TLV block, so subsequently calling size() on such an object (as NFD's StreamTransport does) throws an exception.
You can trigger the same problem with Block{std::numeric_limits<uint32_t>::max()}.size().
I actually noticed this some time ago and thought about using an optional<uint32_t> instead of a special sentinel value for the type, but didn't think it was important.
Updated by Davide Pesavento about 5 years ago
- Subject changed from NFD crashes on unexpected input to Block class mishandles type == 0xffffffff (was: NFD crashes on unexpected input)
Updated by Davide Pesavento about 5 years ago
There is the related question of what is the maximum TLV-TYPE allowed by the packet format, the spec is unclear on that. We'll discuss this matter tomorrow at the platform call if there's time.
Updated by Junxiao Shi about 5 years ago
what is the maximum TLV-TYPE allowed by the packet format
Packet format is pretty clear to me: minimum 0, maximum 264 -1, defined via the range of VAR-NUMBER.
ndn-cxx chooses to support 32-bit only. The obvious solution is for ndn-cxx to declare it supports [0, 232 -2] only and drop others.
Updated by Davide Pesavento about 5 years ago
Junxiao Shi wrote:
what is the maximum TLV-TYPE allowed by the packet format
Packet format is pretty clear to me: minimum 0, maximum 264 -1, defined via the range of VAR-NUMBER.
I think the general sentiment is that such a large number space for TLV types is unnecessary. The question is whether this should be reflected in the spec.
ndn-cxx chooses to support 32-bit only. The obvious solution is for ndn-cxx to declare it supports [0, 232 -2] only and drop others.
A related question is whether an implementation is allowed to support only a subset of numbers and still be considered compliant. I don't think that's the case given the current wording regarding evolvability: http://named-data.net/doc/NDN-packet-spec/current/tlv.html#considerations-for-evolvability-of-tlv-based-encoding
Updated by Junxiao Shi about 5 years ago
- Related to Feature #4895: Limit TLV-TYPE range added
Updated by Davide Pesavento almost 5 years ago
- Status changed from New to Code review
- Assignee set to Davide Pesavento
- Target version set to v0.7
- % Done changed from 0 to 100
Updated by Davide Pesavento almost 5 years ago
- Status changed from Code review to Closed