Skip to content

Commit ffe7b1c

Browse files
ejohnstownphilljj
authored andcommitted
Check the channel packet overhead literal against its expression
CHANNEL_PACKET_OVERHEAD_MAX is a hand-computed copy of CHANNEL_PACKET_OVERHEAD_SZ, needed because the expression bottoms out in wolfCrypt enum constants that #if reads as zero. Nothing tied the two together, so a term added to the expression would leave the #error guarding DEFAULT_MAX_PACKET_SZ silently ineffective. Assert the bound in internal.c, where both are ordinary constant expressions, with a negative-array-size typedef.
1 parent 245baf0 commit ffe7b1c

2 files changed

Lines changed: 17 additions & 15 deletions

File tree

src/internal.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,14 @@
9898

9999
#include <wolfssl/wolfcrypt/coding.h>
100100

101+
102+
/* The #error in internal.h can't compare the two: the expression's terms are
103+
* enum constants that #if reads as zero. Here both are ordinary constant
104+
* expressions, so a term added without bumping the literal fails the build. */
105+
typedef char wolfSSH_channel_overhead_check[
106+
(CHANNEL_PACKET_OVERHEAD_SZ <= CHANNEL_PACKET_OVERHEAD_MAX) ? 1 : -1];
107+
108+
101109
/*
102110
Flags:
103111
HAVE_WC_ECC_SET_RNG

wolfssh/internal.h

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -741,14 +741,11 @@ WOLFSSH_LOCAL int CheckAlgoList(const char* list, byte type);
741741
* block size first to decrypt to find the size of
742742
* the rest of the data. */
743743

744-
/* What a channel data packet carries besides its payload: the transport
745-
* framing, the larger of the two channel data headers (CHANNEL_EXTENDED_DATA),
746-
* the worst-case padding BundlePacket() can pick, and the largest MAC.
747-
* The MAC term is the one that varies, so it is spelled twice: once with
748-
* MAX_HMAC_SZ for the compiler, and once as a literal 64, the largest
749-
* wolfCrypt digest, for the preprocessor. AES_BLOCK_SIZE and MAX_HMAC_SZ are
750-
* wolfCrypt enum constants, which #if reads as zero, so the #error below
751-
* cannot use the macro form. Both come to 4+1+1+8+4+19+64 = 101. */
744+
/* Channel data packet overhead: transport framing, the larger channel data
745+
* header (CHANNEL_EXTENDED_DATA), worst-case BundlePacket() padding, and the
746+
* largest MAC. Spelled twice because AES_BLOCK_SIZE and MAX_HMAC_SZ are enum
747+
* constants that #if reads as zero. The literal is a ceiling: 101 with a
748+
* 64-byte MAC, less with a smaller digest. */
752749
#define CHANNEL_PACKET_OVERHEAD_SZ \
753750
(LENGTH_SZ + PAD_LENGTH_SZ \
754751
+ MSG_ID_SZ + (UINT32_SZ * 2) + LENGTH_SZ \
@@ -757,15 +754,12 @@ WOLFSSH_LOCAL int CheckAlgoList(const char* list, byte type);
757754
#define CHANNEL_PACKET_OVERHEAD_MAX 101
758755

759756
/* Largest channel payload that still fits MAX_PACKET_SZ on the wire, which
760-
* bounds the whole binary packet. Comes to 35000 - 101 = 34899. Derived, not
761-
* a tunable, so it is deliberately not overridable. */
757+
* bounds the whole binary packet. At most 35000 - 101 = 34899. Derived, not
758+
* a tunable, so deliberately not overridable. */
762759
#define MAX_CHANNEL_PACKET_SZ (MAX_PACKET_SZ - CHANNEL_PACKET_OVERHEAD_SZ)
763760

764-
/* wolfSSH_CTX_SetWindowPacketSize() bounds an explicit size by
765-
* MAX_CHANNEL_PACKET_SZ, but a zero there and CtxInit() both take
766-
* DEFAULT_MAX_PACKET_SZ unchecked, so assert the default holds too. Both
767-
* MAX_PACKET_SZ and DEFAULT_MAX_PACKET_SZ are overridable and the default
768-
* path is the common one. */
761+
/* Both sizes are overridable, and CtxInit() takes DEFAULT_MAX_PACKET_SZ
762+
* unchecked, so assert the default frames too. */
769763
#if DEFAULT_MAX_PACKET_SZ > (MAX_PACKET_SZ - CHANNEL_PACKET_OVERHEAD_MAX)
770764
#error "DEFAULT_MAX_PACKET_SZ too large to frame inside MAX_PACKET_SZ"
771765
#endif

0 commit comments

Comments
 (0)