Skip to content

Commit 0f6c386

Browse files
committed
Reject packets with too-little padding
- enforce RFC 4253 section 6 minimum of 4 padding bytes on receive - return WS_BUFFER_E when padding_length is below MIN_PAD_LENGTH - add negative unit test driving DoReceive with a short-padded packet Issue: #1047 (5)
1 parent 265940e commit 0f6c386

2 files changed

Lines changed: 86 additions & 0 deletions

File tree

src/internal.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10291,6 +10291,13 @@ static int DoPacket(WOLFSSH* ssh, byte* bufferConsumed)
1029110291
idx += UINT32_SZ;
1029210292
padSz = buf[idx++];
1029310293

10294+
/* RFC 4253 section 6 requires at least four bytes of padding. */
10295+
if (padSz < MIN_PAD_LENGTH) {
10296+
WLOG(WS_LOG_DEBUG, "Packet padding length %u below minimum %u",
10297+
(word32)padSz, (word32)MIN_PAD_LENGTH);
10298+
return WS_BUFFER_E;
10299+
}
10300+
1029410301
/* check for underflow */
1029510302
if ((word32)(PAD_LENGTH_SZ + padSz + MSG_ID_SZ) > ssh->curSz) {
1029610303
return WS_OVERFLOW_E;

tests/unit.c

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -950,6 +950,78 @@ static int test_DoReceive_VerifyMacFailure(void)
950950
#endif /* WOLFSSH_TEST_INTERNAL && any HMAC SHA variant enabled */
951951

952952

953+
#ifdef WOLFSSH_TEST_INTERNAL
954+
/* Verify DoReceive rejects a binary packet whose padding_length is below the
955+
* RFC 4253 section 6 minimum of four bytes, returning WS_BUFFER_E. The packet
956+
* is delivered in the clear (no cipher, no MAC), matching the pre-key-exchange
957+
* transport, so DoPacket's padding check is what rejects it. */
958+
static int test_DoReceive_RejectsShortPadding(void)
959+
{
960+
WOLFSSH_CTX* ctx = NULL;
961+
WOLFSSH* ssh = NULL;
962+
int ret;
963+
int result = 0;
964+
/* A well-formed MSGID_IGNORE packet carrying an empty string, but with
965+
* padding_length = 1 (below MIN_PAD_LENGTH). Aside from the short padding
966+
* the packet parses cleanly, so the padding check is the only thing that
967+
* can reject it. Layout: uint32 packet_length=7, padding_length=1,
968+
* msgId, uint32 string_len=0, 1 pad byte => 11 bytes total. */
969+
byte pkt[11];
970+
word32 totalLen = (word32)sizeof(pkt);
971+
972+
ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_CLIENT, NULL);
973+
if (ctx == NULL)
974+
return -760;
975+
ssh = wolfSSH_new(ctx);
976+
if (ssh == NULL) {
977+
wolfSSH_CTX_free(ctx);
978+
return -761;
979+
}
980+
981+
pkt[0] = 0; pkt[1] = 0; pkt[2] = 0; pkt[3] = 7; /* packet_length */
982+
pkt[4] = 1; /* padding_length, below MIN_PAD_LENGTH (4) */
983+
pkt[5] = MSGID_IGNORE;
984+
pkt[6] = 0; pkt[7] = 0; pkt[8] = 0; pkt[9] = 0; /* string_len = 0 */
985+
pkt[10] = 0; /* padding */
986+
987+
ssh->peerEncryptId = ID_NONE;
988+
ssh->peerAeadMode = 0;
989+
ssh->peerBlockSz = MIN_BLOCK_SZ;
990+
ssh->peerMacId = ID_NONE;
991+
ssh->peerMacSz = 0;
992+
ssh->peerSeq = 0;
993+
ssh->curSz = 0;
994+
ssh->processReplyState = PROCESS_INIT;
995+
ssh->error = 0;
996+
997+
ShrinkBuffer(&ssh->inputBuffer, 1);
998+
ret = GrowBuffer(&ssh->inputBuffer, totalLen);
999+
if (ret != WS_SUCCESS) {
1000+
result = -762;
1001+
goto done2;
1002+
}
1003+
WMEMCPY(ssh->inputBuffer.buffer, pkt, totalLen);
1004+
ssh->inputBuffer.length = totalLen;
1005+
ssh->inputBuffer.idx = 0;
1006+
1007+
ret = wolfSSH_TestDoReceive(ssh);
1008+
if (ret != WS_FATAL_ERROR) {
1009+
result = -763;
1010+
goto done2;
1011+
}
1012+
if (ssh->error != WS_BUFFER_E) {
1013+
result = -764;
1014+
goto done2;
1015+
}
1016+
1017+
done2:
1018+
wolfSSH_free(ssh);
1019+
wolfSSH_CTX_free(ctx);
1020+
return result;
1021+
}
1022+
#endif /* WOLFSSH_TEST_INTERNAL */
1023+
1024+
9531025
#if defined(WOLFSSH_TEST_INTERNAL) && !defined(WOLFSSH_NO_DH_GEX_SHA256)
9541026

9551027
typedef struct {
@@ -5342,6 +5414,13 @@ int wolfSSH_UnitTest(int argc, char** argv)
53425414
testResult = testResult || unitResult;
53435415
#endif
53445416

5417+
#ifdef WOLFSSH_TEST_INTERNAL
5418+
unitResult = test_DoReceive_RejectsShortPadding();
5419+
printf("DoReceiveRejectsShortPadding: %s\n",
5420+
(unitResult == 0 ? "SUCCESS" : "FAILED"));
5421+
testResult = testResult || unitResult;
5422+
#endif
5423+
53455424
#if defined(WOLFSSH_TEST_INTERNAL) && !defined(WOLFSSH_NO_DH_GEX_SHA256)
53465425
unitResult = test_DhGexGroupValidate();
53475426
printf("DhGexGroupValidate: %s\n",

0 commit comments

Comments
 (0)