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

Validate a custom SSH proto ID string 

wolfSSH_CTX_SetSshProtoIdStr() now rejects a string that is not
CRLF-terminated, exceeds 255 bytes counting the terminator, or
carries a CR or LF in the body. DoKexInit() subtracts the two
terminator bytes from the length when hashing it, so an
unterminated string underflowed the hash length.

Issue: F-10571

partial progress
pull/1247/head
Aidan Keefe 2026-08-31 17:01:14 -06:00 committed by John Safranek
parent 8d6e2b3aba
commit 155a4e1c47
5 changed files with 78 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

51
src/internal.c
View File

@ -1394,6 +1394,7 @@ WOLFSSH_CTX* CtxInit(WOLFSSH_CTX* ctx, byte side, void* heap)
ctx->maxPacketSz = DEFAULT_MAX_PACKET_SZ;
ctx->maxAuthAttempts = DEFAULT_MAX_AUTH_ATTEMPTS;
ctx->sshProtoIdStr = sshProtoIdStr;
ctx->sshProtoIdStrSz = (word32)(sizeof(sshProtoIdStr) - 1);
ctx->algoListKex = cannedKexAlgoNames;
if (side == WOLFSSH_ENDPOINT_CLIENT) {
ctx->algoListKey = cannedKeyAlgoNamesHostKey;
@ -6877,9 +6878,8 @@ static int DoKexInit(WOLFSSH* ssh, byte* buf, word32 len, word32* idx)
}
if (ret == WS_SUCCESS) {
byte SSH_PROTO_EOL_SZ = 2;
strSz = (word32)WSTRLEN(ssh->ctx->sshProtoIdStr) - SSH_PROTO_EOL_SZ;
/* The ID is hashed without its terminator. */
strSz = ssh->ctx->sshProtoIdStrSz - SSH_PROTO_EOL_SZ;
c32toa(strSz, scratchLen);
ret = HashUpdate(hash, hashId, scratchLen, LENGTH_SZ);
}
@ -14626,21 +14626,38 @@ int DoProtoId(WOLFSSH* ssh)
/* Validates a locally configured proto ID string */
int ValidateProtoId(const char* protoIdStr, word32 len)
{
/* Length is checked first: the prefix, terminator, and body checks below
* index and subtract from len. The minimum is the "SSH-2.0-" prefix plus
* one body byte plus CRLF. */
if (protoIdStr == NULL || len < SSH_PROTO_SZ + 3 ||
protoIdStr[len-1] != '\n' || protoIdStr[len-2] != '\r' ||
len > WOLFSSH_PROTOID_LIMIT ||
WSTRNCMP(protoIdStr, sshProtoIdPrefix, SSH_PROTO_SZ) != 0 ||
WSTRNSTR(protoIdStr, "\n", len - 2) != NULL ||
WSTRNSTR(protoIdStr, "\r", len - 2) != NULL) {
WLOG(WS_LOG_ERROR, "Proto Id was invalid: it must start with "
"\"SSH-2.0-\", end in \\r\\n, be no longer than %d bytes, "
"and must not contain \\r or \\n in the body of the line",
WOLFSSH_PROTOID_LIMIT);
word32 i;
if (protoIdStr == NULL ||
len < SSH_PROTO_SZ + 1 + SSH_PROTO_EOL_SZ ||
len > WOLFSSH_PROTOID_LIMIT) {
WLOG(WS_LOG_ERROR, "Proto Id was invalid: it must be between %d and "
"%d bytes, counting the prefix and the terminator",
SSH_PROTO_SZ + 1 + SSH_PROTO_EOL_SZ, WOLFSSH_PROTOID_LIMIT);
return WS_BAD_ARGUMENT;
}
if (WSTRNCMP(protoIdStr, sshProtoIdPrefix, SSH_PROTO_SZ) != 0) {
WLOG(WS_LOG_ERROR, "Proto Id was invalid: it must start with "
"\"SSH-2.0-\"");
return WS_BAD_ARGUMENT;
}
if (protoIdStr[len - 1] != '\n' || protoIdStr[len - 2] != '\r') {
WLOG(WS_LOG_ERROR, "Proto Id was invalid: it must end in \\r\\n");
return WS_BAD_ARGUMENT;
}
for (i = 0; i < len - SSH_PROTO_EOL_SZ; i++) {
byte c = (byte)protoIdStr[i];
if (c < 0x20 || c > 0x7e) {
WLOG(WS_LOG_ERROR, "Proto Id was invalid: byte %u is "
"not printable US-ASCII", i);
return WS_BAD_ARGUMENT;
}
}
return WS_SUCCESS;
}
@ -14655,7 +14672,7 @@ int SendProtoId(WOLFSSH* ssh)
if (ret == WS_SUCCESS) {
WLOG(WS_LOG_DEBUG, "%s", ssh->ctx->sshProtoIdStr);
sshProtoIdStrSz = (word32)WSTRLEN(ssh->ctx->sshProtoIdStr);
sshProtoIdStrSz = ssh->ctx->sshProtoIdStrSz;
ret = GrowBuffer(&ssh->outputBuffer, sshProtoIdStrSz);
}

8
src/ssh.c
View File

@ -3505,19 +3505,23 @@ int wolfSSH_GetMaxAuthAttempts(WOLFSSH* ssh)
int wolfSSH_CTX_SetSshProtoIdStr(WOLFSSH_CTX* ctx,
const char* protoIdStr)
{
word32 protoIdStrSz;
int ret;
WLOG(WS_LOG_DEBUG, "Entering wolfSSH_CTX_SetSshProtoIdStr()");
if (!ctx || !protoIdStr) {
return WS_BAD_ARGUMENT;
}
if ((ret = ValidateProtoId(protoIdStr, (word32)WSTRLEN(protoIdStr))) !=
WS_SUCCESS) {
protoIdStrSz = (word32)WSTRLEN(protoIdStr);
ret = ValidateProtoId(protoIdStr, protoIdStrSz);
if (ret != WS_SUCCESS) {
return ret;
}
ctx->sshProtoIdStr = protoIdStr;
ctx->sshProtoIdStrSz = protoIdStrSz;
return WS_SUCCESS;
}

34
tests/unit.c
View File

@ -609,20 +609,40 @@ static int test_DoProtoId(void)
/* Ensure a malformed local protoId cannot be loaded. */
{
static char tooLongProtoId[257];
static char justRightProtoId[256];
static const struct {
const char* name;
const char* id;
int expectSuccess;
} protoIds[] = {
{ "conforming custom ID", "SSH-2.0-this_is_my_app\r\n", 1 },
{ "wrong version prefix", "SSH-2-this_is_my_app\r\n", 0 },
{ "conforming custom ID", "SSH-2.0-this_is_my_app\r\n", 1 },
{ "shortest valid Id", "SSH-2.0-t\r\n", 1 },
{ "exact len custom ID", justRightProtoId, 1 },
{ "wrong version prefix", "SSH-2-this_is_my_app\r\n", 0 },
{ "bad casing prefix", "sSH-2.0-this_is_my_app\r\n", 0 },
{ "LF terminator only", "SSH-2.0-this_is_my_app\n", 0 },
{ "CR terminator only", "SSH-2.0-this_is_my_app\r", 0 },
{ "empty string", "", 0 },
{ "prefix with no body", "SSH-2.0-\r\n", 0 },
{ "missing prefix", "hello\r\n", 0 },
{ "missing prefix", "hello-this-is\r\n", 0 },
{ "non ascii char", "SSH-2.0-\x90s\r\n", 0 },
{ "Body End in CR", "SSH-2.0-s\r\r\n", 0 },
{ "Body End in TAB", "SSH-2.0-s\t\r\n", 0 },
{ "Body Have bad char", "SSH-2.0-\x02-a\t\r\n", 0 },
{ "too long id", tooLongProtoId, 0 },
};
int pc = (int)(sizeof(protoIds) / sizeof(protoIds[0]));
WMEMSET(tooLongProtoId, 'a', sizeof(tooLongProtoId));
WMEMCPY(tooLongProtoId, "SSH-2.0-", sizeof("SSH-2.0-") - 1);
tooLongProtoId[256] = '\0';
tooLongProtoId[255] = '\n';
tooLongProtoId[254] = '\r';
WMEMSET(justRightProtoId, 'a', sizeof(justRightProtoId));
WMEMCPY(justRightProtoId, "SSH-2.0-", sizeof("SSH-2.0-") - 1);
justRightProtoId[255] = '\0';
justRightProtoId[254] = '\n';
justRightProtoId[253] = '\r';
for (i = 0; i < pc; i++) {
ret = wolfSSH_CTX_SetSshProtoIdStr(clientCtx, protoIds[i].id);
@ -634,6 +654,14 @@ static int test_DoProtoId(void)
: "WS_BAD_ARGUMENT");
failures++;
}
if ((ret == WS_SUCCESS) && clientCtx->sshProtoIdStrSz !=
WSTRLEN(protoIds[i].id)) {
fprintf(stderr,
"\t[protoId %d] \"%s\" FAIL: stored sshProtoIdSz "
"was not retained\n",
i, protoIds[i].name);
failures++;
}
}
}

2
wolfssh/internal.h
View File

@ -587,6 +587,7 @@ enum NameIdType {
#define UINT32_SZ 4
#define LENGTH_SZ UINT32_SZ
#define SSH_PROTO_SZ 8 /* "SSH-2.0-" */
#define SSH_PROTO_EOL_SZ 2 /* "\r\n" */
#define TERMINAL_MODE_SZ 5 /* opcode byte + argument uint32 */
#define TERMINAL_MODES_MAX_SZ 4096
#define TERMINAL_WIDTH_DEFAULT 80 /* used when there is no terminal */
@ -893,6 +894,7 @@ struct WOLFSSH_CTX {
const char* algoListMac;
const char* algoListKeyAccepted;
word32 bannerSz;
word32 sshProtoIdStrSz; /* validated, counting the CRLF */
word32 windowSz;
word32 maxPacketSz;
word32 maxAuthAttempts; /* server cap on failed userauth */

8
wolfssh/ssh.h
View File

@ -665,11 +665,13 @@ WOLFSSH_API int wolfSSH_CTX_SetBanner(WOLFSSH_CTX* ctx, const char* newBanner);
* MUST be between 11 and 255 bytes in length, counting the "SSH-2.0-"
* prefix and the trailing "\r\n"
* MUST end with '\r\n'
* MUST NOT contain '\r' or '\n' in the body
* MUST carry only printable US-ASCII (0x20 - 0x7e) in the body, which
* rules out an embedded '\r' or '\n'
* If these are not adhered to the function will return WS_BAD_ARGUMENT
* and not load the ProtoId in to the WOLFSSH_CTX struct.
* and not load the ProtoId into the WOLFSSH_CTX struct.
* ProtoIdStr is stored by reference and is not copied, so it must remain
* valid for the lifetime of the WOLFSSH_CTX. */
* valid and unmodified for the lifetime of the WOLFSSH_CTX. It is validated
* once, here; a later in-place rewrite of the buffer is not revalidated. */
WOLFSSH_API int wolfSSH_CTX_SetSshProtoIdStr(WOLFSSH_CTX* ctx,
const char* protoIdStr);
/* Set the server-side limit on failed userauth attempts per connection. The