Skip to content

svxreflector: side-effecting calls inside assert() are compiled out under -DNDEBUG (empty UDP datagrams, broken cipher AAD) #828

Description

@MarkRose

Summary

Several side-effecting calls in the reflector are wrapped in assert(...). When the project is compiled with -DNDEBUG (i.e. any optimized/release build, which is what most distributions ship), assert() expands to nothing and the calls — along with their side effects — are removed entirely. This silently breaks reflector behavior in release builds.

Location

src/svxlink/reflector/Reflector.cpp:

415:    assert(header.pack(ss) && msg.pack(ss));   // sendUdpDatagram(), proto >= 3.0 branch
436:    assert(header.pack(ss) && msg.pack(ss));   // sendUdpDatagram(), V2 branch
911:  assert(m_aad.unpack(ss));                    // udpCipherDataReceived()
1017:      assert(aadss.seekg(0));                 // udpDatagramReceived()
1024:      assert(iaad.iv_cntr == 0);             // udpDatagramReceived()  (see note)

Impact (in a -DNDEBUG build)

  • Lines 415 / 436 — the outgoing message is never serialized into ss, so the reflector transmits empty UDP datagrams.
  • Line 911 — the AAD is never unpacked, breaking UDP cipher handling.
  • Line 1017 — the stream is never rewound, so the following InitialAAD unpack reads from the wrong offset.

These are functional-correctness failures that appear only in release builds and not in debug builds, which makes them easy to miss.

Separately, line 1024 asserts on attacker-controlled datagram content (iaad.iv_cntr). In a debug build a malformed initial-AAD datagram aborts the process (a remote DoS); it should be validated and rejected, not asserted.

Suggested fix

Replace each side-effecting assert() with an explicit call whose result is checked and handled, e.g.:

if (!header.pack(ss) || !msg.pack(ss))
{
  std::cout << "*** WARNING: Packing UDP datagram failed for "
            << udp_addr << ":" << udp_port << std::endl;
  return false;
}
if (!m_aad.unpack(ss)) { /* log + ignore datagram */ return true; }
aadss.seekg(0);

and convert assert(iaad.iv_cntr == 0) into a normal validation that drops the datagram when the check fails.

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