Skip to content

packet::find_options out-of-bounds read on an empty (or boundary) options vector #2

Description

@ndss27

Summary

find_options runs a binary search over _message.options with the search upper bound set to size() instead of size() - 1, and with no guard for an empty vector. On a CoAP message that carries no options — a valid and common request shape (RFC 7252 §5.10) — the function indexes _message.options[0] on an empty vector and reads out of bounds, crashing the parser. The same off-by-one allows an out-of-bounds access at the boundary even for non-empty vectors.

Root cause

// src/packet.cc:311
int min = 0 , max = _message.options.size(), mid = 0;   // max should be size()-1
...
// src/packet.cc:324-327
while (min <= max)                                       // empty: 0 <= 0 is true
{
    mid = (min + max) >> 1;                              // mid = 0
    LOG(DEBUGGING,"current option number is ", _message.options[mid].number);
    //                                          ^^^^^^^^^^^^^^^^^^^^^^ options[0] on empty vector → OOB

With max = size():

  • Empty vector: min=0, max=0, loop runs, options[0] is read from a zero-length vector → out-of-bounds.
  • Non-empty boundary: the classic off-by-one (max = size(), min <= max) can drive mid = size(), reading one past the end.

Steps to reproduce

Parse (or construct) a CoAP GET with an empty option list and call
find_options:

packet p;
/* populate p with a valid header and zero options */
size_t qty = 0;
const packet::option_t* opt = p.find_options(URI_PATH, &qty);
// EXPECTED: opt == nullptr, qty == 0
// ACTUAL:   out-of-bounds read at packet.cc:327 (ASan: heap-buffer-overflow / crash)

Expected vs. actual

  • Expected: searching an empty option set returns nullptr with *quantity == 0.
  • Actual: out-of-bounds read; crash.

Impact

Any network peer can send an RFC-compliant option-free request and crash a device that calls find_options during request handling — an unauthenticated remote denial of service. lib-coap-cpp targets embedded/IoT deployments, where this is directly exploitable.

Suggested fix

Guard the empty case and correct the upper bound:

if (_message.options.empty()) { *quantity = 0; return nullptr; }
int min = 0, max = static_cast<int>(_message.options.size()) - 1, mid = 0;

(The inner while (_message.options[++index].number == number) loop at lines 343–346 should also be bounded by index < size()-1 to avoid walking off the end on a run of equal option numbers.)

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions