Skip to content

Commit 8d10dc0

Browse files
wolfsshd: fix Match User/Group directive misparse
1 parent 36e2765 commit 8d10dc0

2 files changed

Lines changed: 403 additions & 34 deletions

File tree

apps/wolfsshd/configuration.c

Lines changed: 74 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -871,44 +871,87 @@ static int HandleChrootDir(WOLFSSHD_CONFIG* conf, const char* value)
871871
}
872872

873873

874-
/* returns WS_SUCCESS on success, helps with adding a restricted case to the
875-
* config */
876-
static int AddRestrictedCase(WOLFSSHD_CONFIG* config, const char* mtch,
877-
const char* value, char** out)
874+
/* returns non-zero if 'c' separates tokens in a Match directive value */
875+
/* Parse the value of a Match directive into the user and group applies-to
876+
* fields of 'config'. The value is a whitespace separated sequence of
877+
* keyword/name pairs, e.g. "User alice Group admins". The token immediately
878+
* following a "User" or "Group" keyword is taken literally as the name and is
879+
* never re-examined as a keyword, so a principal named like the opposite
880+
* keyword (e.g. "Match User Group") is handled by position rather than by a
881+
* substring search. A recognized keyword with no following name (e.g. a bare
882+
* "Match User") is a configuration error so the admin's intent is not silently
883+
* dropped. Unrecognized tokens are ignored to stay lenient toward Match
884+
* criteria that are not yet supported. Returns WS_SUCCESS on success. */
885+
static int ParseMatchCriteria(WOLFSSHD_CONFIG* config, const char* value)
878886
{
879887
int ret = WS_SUCCESS;
880-
char* pt;
888+
const char* pt;
881889

882-
pt = (char*)XSTRSTR(value, mtch);
883-
if (pt != NULL) {
884-
int sz, i;
890+
if (config == NULL || value == NULL) {
891+
ret = WS_BAD_ARGUMENT;
892+
}
885893

886-
pt += (int)XSTRLEN(mtch);
887-
sz = (int)XSTRLEN(pt);
894+
pt = value;
895+
while (ret == WS_SUCCESS && pt != NULL && *pt != '\0') {
896+
const char* tok;
897+
int tokSz;
898+
char** out = NULL;
888899

889-
/* remove spaces between 'mtch' and the user name */
890-
for (i = 0; i < sz; i++) {
891-
if (pt[i] != ' ') break;
900+
/* skip separators preceding the keyword token */
901+
while (WISSPACE((unsigned char)*pt)) {
902+
pt++;
892903
}
893-
if (i == sz) {
894-
wolfSSH_Log(WS_LOG_ERROR,
895-
"[SSHD] No valid input found with Match");
896-
ret = WS_FATAL_ERROR;
904+
if (*pt == '\0') {
905+
break;
897906
}
898907

899-
if (ret == WS_SUCCESS) {
900-
pt += i;
901-
sz -= i;
908+
/* read the keyword token */
909+
tok = pt;
910+
while (*pt != '\0' && !WISSPACE((unsigned char)*pt)) {
911+
pt++;
912+
}
913+
tokSz = (int)(pt - tok);
914+
915+
/* map the keyword to its applies-to field; ignore anything else */
916+
if (tokSz == (int)XSTRLEN("User") &&
917+
WSTRNCMP(tok, "User", tokSz) == 0) {
918+
out = &config->usrAppliesTo;
919+
}
920+
else if (tokSz == (int)XSTRLEN("Group") &&
921+
WSTRNCMP(tok, "Group", tokSz) == 0) {
922+
out = &config->groupAppliesTo;
923+
}
924+
925+
if (out != NULL) {
926+
/* skip separators between the keyword and its name */
927+
while (WISSPACE((unsigned char)*pt)) {
928+
pt++;
929+
}
902930

903-
/* get the actual size of the user name */
904-
for (i = 0; i < sz; i++) {
905-
if (pt[i] == ' ' || pt[i] == '\r' || pt[i] == '\n') break;
931+
/* the next token is the name, taken literally */
932+
tok = pt;
933+
while (*pt != '\0' && !WISSPACE((unsigned char)*pt)) {
934+
pt++;
906935
}
907-
sz = i;
936+
tokSz = (int)(pt - tok);
908937

909-
ret = CreateString(out, pt, sz, config->heap);
938+
if (tokSz == 0) {
939+
wolfSSH_Log(WS_LOG_ERROR,
940+
"[SSHD] Match %s directive is missing a name",
941+
(out == &config->usrAppliesTo) ? "User" : "Group");
942+
ret = WS_FATAL_ERROR;
943+
}
944+
945+
if (ret == WS_SUCCESS) {
946+
/* a repeated keyword replaces the earlier value */
947+
if (*out != NULL) {
948+
FreeString(out, config->heap);
949+
}
950+
ret = CreateString(out, tok, tokSz, config->heap);
951+
}
910952
}
911953
}
954+
912955
return ret;
913956
}
914957

@@ -932,16 +975,9 @@ static int HandleMatch(WOLFSSHD_CONFIG** conf, const char* value, int valueSz)
932975
}
933976
}
934977

935-
/* users the settings apply to */
978+
/* parse the User/Group criteria the settings apply to */
936979
if (ret == WS_SUCCESS) {
937-
ret = AddRestrictedCase(newConf, "User", value,
938-
&newConf->usrAppliesTo);
939-
}
940-
941-
/* groups the settings apply to */
942-
if (ret == WS_SUCCESS) {
943-
ret = AddRestrictedCase(newConf, "Group", value,
944-
&newConf->groupAppliesTo);
980+
ret = ParseMatchCriteria(newConf, value);
945981
}
946982

947983
/* @TODO handle , separated user/group list */
@@ -951,6 +987,10 @@ static int HandleMatch(WOLFSSHD_CONFIG** conf, const char* value, int valueSz)
951987
(*conf)->next = newConf;
952988
(*conf) = newConf;
953989
}
990+
else {
991+
/* newConf was allocated but not linked into the list; free it */
992+
wolfSSHD_ConfigFree(newConf);
993+
}
954994

955995
(void)valueSz;
956996
return ret;

0 commit comments

Comments
 (0)