Bug #1970: segfault in Exclude::wireDecode - ndn-cxx - NDN project issue tracking system

Actions

Copy link

Bug #1970

closed

segfault in Exclude::wireDecode

Added by Tai-Lin Chu about 10 years ago. Updated about 10 years ago.

Status:

Closed

Priority:

High

Assignee:

Junxiao Shi

Category:

Base

Target version:

v0.3

Start date:

09/06/2014

Due date:

% Done:

100%

Estimated time:

1.00 h

Description

Snippet to reproduce:

Block block(tlv::Exclude);
try {
  Exclude exclude(block);
}
catch(tlv::Error&) {
}

Expected: Exclude throws a tlv::Error which can be caught

Actual: segmentation fault

Actions

Copy link

Updated by Alex Afanasyev about 10 years ago

Please give us specific example/code on how to reproduce the problem.

nfd has some protection against malformed packets and the cases you describe should not have caused crash.

Actions

Copy link

Updated by Tai-Lin Chu about 10 years ago

This is the byte sequence that I send. I already enable All logs in the config, but it still shows nothing.

([]uint8) (len=54 cap=64) {
 00000000  05 34 07 10 08 03 6e 64  6e 08 03 65 64 75 08 04  |.4....ndn..edu..|
 00000010  75 63 6c 61 09 13 0d 01  00 0e 01 00 0f 04 07 00  |ucla............|
 00000020  1d 00 10 00 11 01 00 12  00 0a 04 dd 64 58 92 0b  |............dX..|
 00000030  01 00 0c 02 0f a0                                 |......|
}

Actions

Copy link

Updated by Junxiao Shi about 10 years ago

Description updated (diff)

Backtrace:

#0  0x000000000049072e in ndn::Block::type (this=0x0)
    at /usr/local/include/ndn-cxx/encoding/block.hpp:355
#1  0x00000000006253f9 in ndn::Exclude::wireDecode (this=0xaa69d0, wire=...)
    at ../src/exclude.hpp:322
#2  0x0000000000625af7 in ndn::Selectors::wireDecode (this=0xaa68a8, wire=...)
    at ../src/selectors.hpp:337
#3  0x000000000062492c in ndn::Interest::wireDecode (this=0xaa6830, wire=...)
    at ../src/interest.cpp:278
#4  0x00000000004be233 in nfd::LocalFace::decodeAndDispatchInput (
    this=0x117bbb0, element=...) at ../daemon/face/local-face.hpp:151
#5  0x00000000005976a6 in nfd::StreamFace<boost::asio::local::stream_protocol, nfd::LocalFace>::handleReceive (this=0x117bbb0, error=..., nBytesReceived=54)
    at ../daemon/face/stream-face.hpp:306

Actions

Copy link

Updated by Junxiao Shi about 10 years ago

Category set to Faces
Assignee set to Davide Pesavento
Target version set to v0.3
Estimated time set to 5.00 h

Here's a breakdown of this packet.

05 34 Interest
   07 10 Name
      08 03 NameComponent
         6e 64 6e
      08 03 NameComponent
         65 64 75
      08 04 NameComponent
         75 63 6c 61
   09 13 Selectors
      0d 01 MinSuffixComponents
         00
      0e 01 MaxSuffixComponents
         00
      0f 04 PublisherPublicKeyLocator
         07 00 Name
         1d 00 KeyDigest
      10 00 Exclude
      11 01 ChildSelector
         00
      12 00 MustBeFresh
   0a 04 Nonce
      dd 64 58 92
   0b 01 Scope
      00
   0c 02 InterestLifetime
      0f a0

This packet is invalid in several places:

PublisherPublicKeyLocator type should never be used. The PublisherPublicKeyLocator selector uses KeyLocator type.
Name and KeyDigest cannot appear together under KeyLocator.
Exclude element cannot be empty.

In any case, this bug is about the missing/incomplete error handling in the face.
The same problem appears in all of: TcpFace, TcpLocalFace, UdpFace, UnixStreamFace.
I haven't tested other face types.

Actions

Copy link

Updated by Davide Pesavento about 10 years ago

Junxiao Shi wrote:

In any case, this bug is about the missing/incomplete error handling in the face.

I disagree. There's a null pointer dereference in ndn::Exclude::wireDecode, I don't see what the face could possibly do about it.

IIUC, the aformentioned method assumes that the Exclude is well-formed (i.e. contains at least one element) and dereferences (line 322) the iterator i returned by elements_begin() without checking its validity.

IMHO this is a bug in ndn-cxx.

Actions

Copy link

Updated by Junxiao Shi about 10 years ago

Project changed from NFD to ndn-cxx
Subject changed from malformed interest could crash nfd to segfault in Exclude::wireDecode
Description updated (diff)
Category changed from Faces to Base
Assignee changed from Davide Pesavento to Junxiao Shi
Priority changed from Normal to High
Target version changed from v0.3 to v0.3
Estimated time changed from 5.00 h to 1.00 h