WolfSSL
/
wolfssh
mirror of https://github.com/wolfSSL/wolfssh.git
1
0
Fork 0
Code Issues Releases Wiki Activity

Frame DoPacket from the validated packet length 

DoPacket stepped to the next packet using payloadIdx, which handlers
set to however much they read. The default case reads none of an
unimplemented message's payload, leaving the cursor short by that much.

- Advance inputBuffer.idx by UINT32_SZ + curSz from the packet start,
  the length DoReceive already bounds-checked, so a handler that
  ignores trailing bytes cannot move the next packet's start.
- Snapshot curSz on entry beside the packet start, so the frame is
  computed entirely from entry-time state. Reading it back after the
  handler switch would describe the next packet if a handler ever
  re-entered the receive path.
- Covers DoIgnore, DoDebug, DoUnimplemented, DoChannelSuccess and
  DoChannelFailure, which are all short on a padded payload.
- Clamp to the buffer length on the WS_BUFFER_E path.
- unit.c pins the cursor across an unimplemented message, driving
  DoPacket through a new wolfSSH_TestDoPacket() hook. The ShrinkBuffer()
  noted below zeroes the cursor, so DoReceive() cannot be in the path.

Note the short cursor is not currently observable: DoReceive calls
ShrinkBuffer() with forcedFree, which drops the rest of the buffer
after every packet. This is hardening, not a live desync.

Issue: F-8825
pull/1171/head
John Safranek 2026-08-17 13:43:12 -07:00 committed by philljj
parent 4d9f6a9721
commit fd91af61c3
3 changed files with 163 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
src/internal.c
View File

@ -12106,7 +12106,9 @@ static int DoChannelExtendedData(WOLFSSH* ssh,
static int DoPacket(WOLFSSH* ssh, byte* bufferConsumed)
{
byte* buf = (byte*)ssh->inputBuffer.buffer;
word32 idx = ssh->inputBuffer.idx;
word32 pktStart = ssh->inputBuffer.idx;
word32 pktSz = ssh->curSz;
word32 idx = pktStart;
word32 len = ssh->inputBuffer.length;
word32 payloadSz;
byte padSz;
@ -12129,11 +12131,11 @@ static int DoPacket(WOLFSSH* ssh, byte* bufferConsumed)
}
/* check for underflow */
if ((word32)(PAD_LENGTH_SZ + padSz + MSG_ID_SZ) > ssh->curSz) {
if ((word32)(PAD_LENGTH_SZ + padSz + MSG_ID_SZ) > pktSz) {
return WS_OVERFLOW_E;
}
payloadSz = ssh->curSz - PAD_LENGTH_SZ - padSz - MSG_ID_SZ;
payloadSz = pktSz - PAD_LENGTH_SZ - padSz - MSG_ID_SZ;
msg = buf[idx++];
/* At this point, payload starts at "buf + idx". */
@ -12362,15 +12364,17 @@ static int DoPacket(WOLFSSH* ssh, byte* bufferConsumed)
/* if the auth is still pending, don't discard the packet data */
if (ret != WS_AUTH_PENDING) {
if (payloadSz > 0) {
idx += payloadIdx;
if (idx + padSz > len) {
WLOG(WS_LOG_DEBUG, "Not enough data in buffer for pad.");
ret = WS_BUFFER_E;
}
/* Step over the packet using the length DoReceive already validated,
* not payloadIdx. A handler is free to read less than the payload it
* was handed -- the default case above reads none of it -- and that
* must not decide where the next packet begins. pktStart and pktSz
* are both from entry, so a handler cannot move the frame either. */
idx = pktStart + UINT32_SZ + pktSz;
if (idx > len) {
WLOG(WS_LOG_DEBUG, "Not enough data in buffer for packet.");
ret = WS_BUFFER_E;
idx = len;
}
idx += padSz;
ssh->inputBuffer.idx = idx;
ssh->peerSeq++;
ssh->rxMsgCount++;
@ -22921,6 +22925,11 @@ int wolfSSH_TestDoReceive(WOLFSSH* ssh)
return DoReceive(ssh);
}
int wolfSSH_TestDoPacket(WOLFSSH* ssh, byte* bufferConsumed)
{
return DoPacket(ssh, bufferConsumed);
}
int wolfSSH_TestDoUserAuthBanner(WOLFSSH* ssh, byte* buf, word32 len,
word32* idx)
{

141
tests/unit.c
View File

@ -1279,6 +1279,140 @@ static word32 BuildMacTestPacketPrefix(byte msgId, byte padLen,
#endif
/* The test below drives a server-side session. With NO_WOLFSSH_SERVER the
* message filter has no server branch, so every message on such a session is
* refused and the test cannot run. */
#if defined(WOLFSSH_TEST_INTERNAL) && !defined(NO_WOLFSSH_SERVER)
/* Swallow the SSH_MSG_UNIMPLEMENTED that DoPacket sends back, so its send
* does not decide the result of the test. */
static int SinkIoSendUnimplemented(WOLFSSH* ssh, void* buf, word32 sz,
void* ctx)
{
(void)ssh; (void)buf; (void)ctx;
return (int)sz;
}
/* Two back-to-back packets in one buffer. DoPacket() is driven directly:
* DoReceive() force-frees the buffer after every packet, zeroing the cursor
* under test. */
static const byte s_unimplStream[] = {
/* unimplemented message, 8 bytes of payload that no handler reads */
0x00, 0x00, 0x00, 0x10, /* packetSz = 16 */
0x06, /* padSz = 6 */
0x0A, /* msgId = 10 */
0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, /* payload */
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* padding */
/* MSGID_IGNORE carrying a zero-length string */
0x00, 0x00, 0x00, 0x0C, /* packetSz = 12 */
0x06, /* padSz = 6 */
0x02, /* msgId = IGNORE */
0x00, 0x00, 0x00, 0x00, /* string, len 0 */
0x00, 0x00, 0x00, 0x00, 0x00, 0x00 /* padding */
};
/* Message ID 10 is unassigned and always allowed, so it reaches DoPacket's
* default case, which reads none of its 8 payload bytes. DoPacket must step
* over them itself. Checked by the cursor it leaves, and by the length peeked
* there being the next packet's. Framing from payloadIdx lands 8 short. */
static int test_DoPacket_UnimplementedConsumesPayload(void)
{
WOLFSSH_CTX* ctx = NULL;
WOLFSSH* ssh = NULL;
int result = 0;
int ret;
int i;
/* packetSz of each packet in s_unimplStream, in order */
static const word32 pktSizes[] = { 16, 12 };
ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL);
if (ctx == NULL)
return -220;
wolfSSH_SetIOSend(ctx, SinkIoSendUnimplemented);
ssh = wolfSSH_new(ctx);
if (ssh == NULL) { result = -221; goto done; }
/* Past user auth, so the connection-layer message IDs are allowed. */
ssh->acceptState = ACCEPT_SERVER_USERAUTH_SENT;
ssh->peerEncryptId = ID_NONE;
ssh->peerMacId = ID_NONE;
ssh->peerAeadMode = 0;
ssh->peerBlockSz = MIN_BLOCK_SZ;
ssh->peerMacSz = 0;
ssh->peerSeq = 0;
ssh->error = 0;
/* Both packets in the buffer at once. */
ShrinkBuffer(&ssh->inputBuffer, 1);
if (GrowBuffer(&ssh->inputBuffer, (word32)sizeof(s_unimplStream))
!= WS_SUCCESS) {
result = -222;
goto done;
}
WMEMCPY(ssh->inputBuffer.buffer, s_unimplStream, sizeof(s_unimplStream));
ssh->inputBuffer.length = (word32)sizeof(s_unimplStream);
ssh->inputBuffer.idx = 0;
for (i = 0; i < (int)(sizeof(pktSizes) / sizeof(pktSizes[0])); i++) {
word32 pktStart = ssh->inputBuffer.idx;
const byte* lenField = ssh->inputBuffer.buffer + pktStart;
word32 curSz;
byte bufferConsumed = 0;
/* The peek DoReceive does; a short cursor reads leftover payload. */
curSz = ((word32)lenField[0] << 24) | ((word32)lenField[1] << 16)
| ((word32)lenField[2] << 8) | (word32)lenField[3];
if (curSz != pktSizes[i]) {
printf("DoPacket[%d]: packetSz=%u at cursor %u, expected %u\n",
i, curSz, pktStart, pktSizes[i]);
result = -223;
goto done;
}
ssh->curSz = curSz;
ret = wolfSSH_TestDoPacket(ssh, &bufferConsumed);
if (ret != WS_SUCCESS) {
printf("DoPacket[%d]: ret=%d, error=%d\n", i, ret, ssh->error);
result = -224;
goto done;
}
if (!bufferConsumed) {
printf("DoPacket[%d]: packet not consumed\n", i);
result = -225;
goto done;
}
/* The whole packet, no more and no less. */
if (ssh->inputBuffer.idx != pktStart + UINT32_SZ + curSz) {
printf("DoPacket[%d]: cursor at %u, expected %u\n", i,
ssh->inputBuffer.idx, pktStart + UINT32_SZ + curSz);
result = -226;
goto done;
}
if (ssh->peerSeq != (word32)(i + 1)) {
printf("DoPacket[%d]: peerSeq=%u, expected %d\n", i,
ssh->peerSeq, i + 1);
result = -227;
goto done;
}
}
/* Buffer exactly spent. */
if (ssh->inputBuffer.idx != (word32)sizeof(s_unimplStream)) {
printf("DoPacket: %u of %u bytes consumed\n", ssh->inputBuffer.idx,
(word32)sizeof(s_unimplStream));
result = -228;
goto done;
}
done:
wolfSSH_free(ssh);
wolfSSH_CTX_free(ctx);
return result;
}
#endif /* WOLFSSH_TEST_INTERNAL && !NO_WOLFSSH_SERVER */
#if defined(WOLFSSH_TEST_INTERNAL) && \
(!defined(WOLFSSH_NO_HMAC_SHA1) || \
!defined(WOLFSSH_NO_HMAC_SHA1_96) || \
@ -16544,6 +16678,13 @@ int wolfSSH_UnitTest(int argc, char** argv)
testResult = testResult || unitResult;
#endif
#if defined(WOLFSSH_TEST_INTERNAL) && !defined(NO_WOLFSSH_SERVER)
unitResult = test_DoPacket_UnimplementedConsumesPayload();
printf("DoPacketUnimplemented: %s\n",
(unitResult == 0 ? "SUCCESS" : "FAILED"));
testResult = testResult || unitResult;
#endif
#if defined(WOLFSSH_TEST_INTERNAL) && \
(!defined(WOLFSSH_NO_HMAC_SHA1) || \
!defined(WOLFSSH_NO_HMAC_SHA1_96) || \

2
wolfssh/internal.h
View File

@ -1810,6 +1810,8 @@ enum WS_MessageIdLimits {
WOLFSSH_API int wolfSSH_TestIsMessageAllowed(WOLFSSH* ssh, byte msg,
byte state);
WOLFSSH_API int wolfSSH_TestDoReceive(WOLFSSH* ssh);
WOLFSSH_API int wolfSSH_TestDoPacket(WOLFSSH* ssh,
byte* bufferConsumed);
WOLFSSH_API int wolfSSH_TestDoUserAuthBanner(WOLFSSH* ssh, byte* buf,
word32 len, word32* idx);
WOLFSSH_API int wolfSSH_TestPrepareUserAuthRequestPassword(WOLFSSH* ssh,