[RFC non-compliance] Server sends a port in SSH_MSG_REQUEST_SUCCESS for a non-dynamic tcpip-forward (RFC 4254 §7.1)

[LCBH]

Component: wolfSSH server, TCP/IP forwarding (tcpip-forward global request)
Affected: v1.5.0-stable (release) and current master (ba6bc667,
v1.5.0-stable-533-gba6bc667) — confirmed present on both.
Type: RFC 4254 §7.1 conformance / interoperability. Severity: low.
Found by: differential fuzzing with puffin
(sshpuffin, wolfSSH vs libssh vs OpenSSH behaviour).

Summary

When a wolfSSH server accepts an SSH_MSG_GLOBAL_REQUEST "tcpip-forward" whose
requested bind port is non-zero (an explicit port, not dynamic allocation) and
want_reply is TRUE, it replies with SSH_MSG_REQUEST_SUCCESS followed by a
uint32 bound-port field. RFC 4254 §7.1 specifies that trailing port field only
for a port-0 (dynamic) request; for an explicit non-zero port the reply must be
a bare SSH_MSG_REQUEST_SUCCESS with no response-specific data. OpenSSH and libssh
both send the bare reply.

RFC 4254 §7.1

"If a client passes 0 as port number to bind and has 'want reply' as TRUE, then
the server allocates the next available unprivileged port number and replies
with the following message; otherwise, there is no response-specific data.
byte SSH_MSG_REQUEST_SUCCESS
uint32 port that was bound on the server"

The wire message wolfSSH emits for a non-zero request does not match the format the
spec defines for that case ("otherwise, there is no response-specific data").

Reproduction

A raw SSH client (curve25519 + aes256-gcm), authenticates (echoserver sample
credential jill:upthehill), then sends
SSH_MSG_GLOBAL_REQUEST "tcpip-forward" want_reply=TRUE "127.0.0.1" <port> and
inspects the REQUEST_SUCCESS body. Against a stock echoserver built --enable-fwd:

requested port 8022 (explicit, non-zero):
  reply: 51 00 00 1f 56   = SSH_MSG_REQUEST_SUCCESS + uint32 0x00001f56 (8022)   <-- WRONG (extra port)
requested port 0 (dynamic):
  reply: 51 00 00 9e e9   = SSH_MSG_REQUEST_SUCCESS + uint32 0x00009ee9 (40681)  <-- correct (allocated port)

For comparison, OpenSSH sshd and libssh both reply with a bare 51 (no trailing
bytes) for the non-zero case.

Root cause (master src/internal.c)

In the global-request handler, a successful non-cancel tcpip-forward with
want_reply always calls SendGlobalRequestFwdSuccess(ssh, 1, bindPort):

if (wantReply) {
    if (ret == WS_SUCCESS) {
        if (isCancel) {
            ret = SendRequestSuccess(ssh, 1);              /* bare, no port */
        } else {
            ret = SendGlobalRequestFwdSuccess(ssh, 1, bindPort);  /* always packs port */
        }
    } else {
        ret = SendRequestSuccess(ssh, 0);
    }
}

and SendGlobalRequestFwdSuccess appends the port unconditionally:

if (success) {
    output[idx++] = MSGID_REQUEST_SUCCESS;
    c32toa(port, output + idx);   /* emitted regardless of whether the request was port-0 */
    idx += UINT32_SZ;
}

There is no bindPort == 0 guard at the reply site. (master already tracks
portPending = (bindPort == 0) in its forwarding bookkeeping, but that flag is not
consulted here.) For reference, libssh guards correctly in
ssh_message_global_request_reply_success (src/server.c): it packs the port only
if (msg->global_request.type == SSH_GLOBAL_REQUEST_TCPIP_FORWARD && msg->global_request.bind_port == 0); OpenSSH (serverloop.c) packs it only
if (allocated_listen_port != 0).

Suggested fix

Choose the reply builder on whether the client requested a dynamic port:

if (!isCancel && bindPort == 0)
    /* dynamic request: return the port the server actually bound */
    ret = SendGlobalRequestFwdSuccess(ssh, 1, /* the bound port */);
else
    /* explicit port, or cancel: bare reply, no response-specific data */
    ret = SendRequestSuccess(ssh, 1);

i.e. emit the uint32 port only for a port-0 (dynamic) request (passing whatever
port the server actually bound), matching RFC 4254 §7.1, OpenSSH, and libssh.
(Illustrative — the exact bound-port value comes from wolfSSH's own forwarding
bookkeeping, e.g. the portPending/bindPort state it already maintains.)

Impact

Low — interoperability/conformance, not a security issue. A strict RFC-4254 client
that expects exactly a bare SSH_MSG_REQUEST_SUCCESS for its non-zero forward
receives 4 unexpected trailing bytes; most clients ignore trailing bytes, so
breakage is unlikely but possible with a strict parser. Requires an authenticated
session with forwarding enabled. No memory-safety impact (the parse is bounded;
ASAN-clean across the fuzzing campaign).

Notes

RFC 4254 §7.1 states this behaviour descriptively ("then … otherwise …") rather
than with an RFC 2119 MUST, so this is a conformance/interop deviation from the
spec's defined message format rather than a normative-MUST violation. Reported for
robustness parity with OpenSSH and libssh.

Acknowledgements

This bug was found thanks to the tlspuffin fuzzer designed and developed by the tlspuffin team:

Tom Gouville - Loria, Inria
Lucca Hirschi - Loria, Inria
Steve Kremer - Loria, Inria
Nataël Baffou - Loria, Inria
Olivier Demengeon - Loria, Inria

[ejohnstown]

Thanks for the bug report. We're stacking up a bunch of forwarding fixes, I'll make sure this gets in there, too.