Skip to content

Commit

Permalink
Add missing NULL checks for mandatory fields in protobuf messages.
Browse files Browse the repository at this point in the history 
Also no longer reject an InfoMessage with an unknown value_case,
just log and ignore it.
  • Loading branch information
@millert
millert committed Sep 28, 2022
1 parent 9150423 commit e6f2ad0
Show file tree
Hide file tree
Showing 4 changed files with 111 additions and 99 deletions.
154 changes: 60 additions & 94 deletions logsrvd/iolog_writer.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,22 +54,28 @@

#include "logsrvd.h"

static inline bool
has_numval(InfoMessage *info)
{
return info->value_case == INFO_MESSAGE__VALUE_NUMVAL;
}

static inline bool
has_strval(InfoMessage *info)
static bool
type_matches(InfoMessage *info, const char *source,
InfoMessage__ValueCase value_case)
{
return info->value_case == INFO_MESSAGE__VALUE_STRVAL;
}
const void *val = info->u.strval; /* same for strlistval */
debug_decl(type_matches, SUDO_DEBUG_UTIL);

static inline bool
has_strlistval(InfoMessage *info)
{
return info->value_case == INFO_MESSAGE__VALUE_STRLISTVAL;
if (info->key == NULL) {
sudo_warnx(U_("%s: protocol error: NULL key"), source);
debug_return_bool(false);
}
if (info->value_case != value_case) {
sudo_warnx(U_("%s: protocol error: wrong type for %s"),
source, info->key);
debug_return_bool(false);
}
if (value_case != INFO_MESSAGE__VALUE_NUMVAL && val == NULL) {
sudo_warnx(U_("%s: protocol error: NULL value found in %s"),
source, info->key);
debug_return_bool(false);
}
debug_return_bool(true);
}

/*
Expand Down Expand Up @@ -159,210 +165,170 @@ evlog_new(TimeSpec *submit_time, InfoMessage **info_msgs, size_t infolen,
switch (key[0]) {
case 'c':
if (strcmp(key, "columns") == 0) {
if (!has_numval(info)) {
sudo_warnx(U_("%s: protocol error: wrong type for %s"),
source, "columns");
} else if (info->u.numval <= 0 || info->u.numval > INT_MAX) {
errno = ERANGE;
sudo_warn(U_("%s: %s"), source, "columns");
} else {
evlog->columns = info->u.numval;
if (type_matches(info, source, INFO_MESSAGE__VALUE_NUMVAL)) {
if (info->u.numval <= 0 || info->u.numval > INT_MAX) {
errno = ERANGE;
sudo_warn(U_("%s: %s"), source, "columns");
} else {
evlog->columns = info->u.numval;
}
}
continue;
}
if (strcmp(key, "command") == 0) {
if (has_strval(info)) {
if (type_matches(info, source, INFO_MESSAGE__VALUE_STRVAL)) {
if ((evlog->command = strdup(info->u.strval)) == NULL) {
sudo_warnx(U_("%s: %s"), __func__,
U_("unable to allocate memory"));
goto bad;
}
} else {
sudo_warnx(U_("%s: protocol error: wrong type for %s"),
source, "command");
}
continue;
}
break;
case 'l':
if (strcmp(key, "lines") == 0) {
if (!has_numval(info)) {
sudo_warnx(U_("%s: protocol error: wrong type for %s"),
source, "lines");
} else if (info->u.numval <= 0 || info->u.numval > INT_MAX) {
errno = ERANGE;
sudo_warn(U_("%s: %s"), source, "lines");
} else {
evlog->lines = info->u.numval;
if (type_matches(info, source, INFO_MESSAGE__VALUE_NUMVAL)) {
if (info->u.numval <= 0 || info->u.numval > INT_MAX) {
errno = ERANGE;
sudo_warn(U_("%s: %s"), source, "lines");
} else {
evlog->lines = info->u.numval;
}
}
continue;
}
break;
case 'r':
if (strcmp(key, "runargv") == 0) {
if (has_strlistval(info)) {
if (type_matches(info, source, INFO_MESSAGE__VALUE_STRLISTVAL)) {
evlog->argv = strlist_copy(info->u.strlistval);
if (evlog->argv == NULL)
goto bad;
} else {
sudo_warnx(U_("%s: protocol error: wrong type for %s"),
source, "runargv");
}
continue;
}
if (strcmp(key, "runchroot") == 0) {
if (has_strval(info)) {
if (type_matches(info, source, INFO_MESSAGE__VALUE_STRVAL)) {
if ((evlog->runchroot = strdup(info->u.strval)) == NULL) {
sudo_warnx(U_("%s: %s"), __func__,
U_("unable to allocate memory"));
goto bad;
}
} else {
sudo_warnx(U_("%s: protocol error: wrong type for %s"),
source, "runchroot");
}
continue;
}
if (strcmp(key, "runcwd") == 0) {
if (has_strval(info)) {
if (type_matches(info, source, INFO_MESSAGE__VALUE_STRVAL)) {
if ((evlog->runcwd = strdup(info->u.strval)) == NULL) {
sudo_warnx(U_("%s: %s"), __func__,
U_("unable to allocate memory"));
goto bad;
}
} else {
sudo_warnx(U_("%s: protocol error: wrong type for %s"),
source, "runcwd");
}
continue;
}
if (strcmp(key, "runenv") == 0) {
if (has_strlistval(info)) {
if (type_matches(info, source, INFO_MESSAGE__VALUE_STRLISTVAL)) {
evlog->envp = strlist_copy(info->u.strlistval);
if (evlog->envp == NULL)
goto bad;
} else {
sudo_warnx(U_("%s: protocol error: wrong type for %s"),
source, "runenv");
}
continue;
}
if (strcmp(key, "rungid") == 0) {
if (!has_numval(info)) {
sudo_warnx(U_("%s: protocol error: wrong type for %s"),
source, "rungid");
} else if (info->u.numval < 0 || info->u.numval > INT_MAX) {
errno = ERANGE;
sudo_warn(U_("%s: %s"), source, "rungid");
} else {
evlog->rungid = info->u.numval;
if (type_matches(info, source, INFO_MESSAGE__VALUE_NUMVAL)) {
if (info->u.numval < 0 || info->u.numval > INT_MAX) {
errno = ERANGE;
sudo_warn(U_("%s: %s"), source, "rungid");
} else {
evlog->rungid = info->u.numval;
}
}
continue;
}
if (strcmp(key, "rungroup") == 0) {
if (has_strval(info)) {
if (type_matches(info, source, INFO_MESSAGE__VALUE_STRVAL)) {
if ((evlog->rungroup = strdup(info->u.strval)) == NULL) {
sudo_warnx(U_("%s: %s"), __func__,
U_("unable to allocate memory"));
goto bad;
}
} else {
sudo_warnx(U_("%s: protocol error: wrong type for %s"),
source, "rungroup");
}
continue;
}
if (strcmp(key, "runuid") == 0) {
if (!has_numval(info)) {
sudo_warnx(U_("%s: protocol error: wrong type for %s"),
source, "runuid");
} else if (info->u.numval < 0 || info->u.numval > INT_MAX) {
errno = ERANGE;
sudo_warn(U_("%s: %s"), source, "runuid");
} else {
evlog->runuid = info->u.numval;
if (type_matches(info, source, INFO_MESSAGE__VALUE_NUMVAL)) {
if (info->u.numval < 0 || info->u.numval > INT_MAX) {
errno = ERANGE;
sudo_warn(U_("%s: %s"), source, "runuid");
} else {
evlog->runuid = info->u.numval;
}
}
continue;
}
if (strcmp(key, "runuser") == 0) {
if (has_strval(info)) {
if (type_matches(info, source, INFO_MESSAGE__VALUE_STRVAL)) {
if ((evlog->runuser = strdup(info->u.strval)) == NULL) {
sudo_warnx(U_("%s: %s"), __func__,
U_("unable to allocate memory"));
goto bad;
}
} else {
sudo_warnx(U_("%s: protocol error: wrong type for %s"),
source, "runuser");
}
continue;
}
break;
case 's':
if (strcmp(key, "submitcwd") == 0) {
if (has_strval(info)) {
if (type_matches(info, source, INFO_MESSAGE__VALUE_STRVAL)) {
if ((evlog->cwd = strdup(info->u.strval)) == NULL) {
sudo_warnx(U_("%s: %s"), __func__,
U_("unable to allocate memory"));
goto bad;
}
} else {
sudo_warnx(U_("%s: protocol error: wrong type for %s"),
source, "submitcwd");
}
continue;
}
if (strcmp(key, "submitgroup") == 0) {
if (has_strval(info)) {
if (type_matches(info, source, INFO_MESSAGE__VALUE_STRVAL)) {
if ((evlog->submitgroup = strdup(info->u.strval)) == NULL) {
sudo_warnx(U_("%s: %s"), __func__,
U_("unable to allocate memory"));
goto bad;
}
} else {
sudo_warnx(U_("%s: protocol error: wrong type for %s"),
source, "submitgroup");
}
continue;
}
if (strcmp(key, "submithost") == 0) {
if (has_strval(info)) {
if (type_matches(info, source, INFO_MESSAGE__VALUE_STRVAL)) {
if ((evlog->submithost = strdup(info->u.strval)) == NULL) {
sudo_warnx(U_("%s: %s"), __func__,
U_("unable to allocate memory"));
goto bad;
}
} else {
sudo_warnx(U_("%s: protocol error: wrong type for %s"),
source, "submithost");
}
continue;
}
if (strcmp(key, "submituser") == 0) {
if (has_strval(info)) {
if (type_matches(info, source, INFO_MESSAGE__VALUE_STRVAL)) {
if ((evlog->submituser = strdup(info->u.strval)) == NULL) {
sudo_warnx(U_("%s: %s"), __func__,
U_("unable to allocate memory"));
goto bad;
}
} else {
sudo_warnx(U_("%s: protocol error: wrong type for %s"),
source, "submituser");
}
continue;
}
break;
case 't':
if (strcmp(key, "ttyname") == 0) {
if (has_strval(info)) {
if (type_matches(info, source, INFO_MESSAGE__VALUE_STRVAL)) {
if ((evlog->ttyname = strdup(info->u.strval)) == NULL) {
sudo_warnx(U_("%s: %s"), __func__,
U_("unable to allocate memory"));
goto bad;
}
} else {
sudo_warnx(U_("%s: protocol error: wrong type for %s"),
source, "ttyname");
}
continue;
}
Expand Down
33 changes: 32 additions & 1 deletion logsrvd/logsrvd.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,12 @@ handle_exit(ExitMessage *msg, uint8_t *buf, size_t len,
debug_return_bool(false);
}

/* Check that message is valid. */
if (msg->run_time == NULL) {
sudo_warnx(U_("%s: %s"), source, U_("invalid ExitMessage"));
closure->errstr = _("invalid ExitMessage");
debug_return_bool(false);
}
sudo_debug_printf(SUDO_DEBUG_INFO, "%s: received ExitMessage from %s",
source, __func__);

Expand Down Expand Up @@ -564,6 +570,13 @@ handle_restart(RestartMessage *msg, uint8_t *buf, size_t len,
closure->errstr = _("state machine error");
debug_return_bool(false);
}

/* Check that message is valid. */
if (msg->log_id == NULL || msg->resume_point == NULL) {
sudo_warnx(U_("%s: %s"), source, U_("invalid RestartMessage"));
closure->errstr = _("invalid RestartMessage");
debug_return_bool(false);
}
sudo_debug_printf(SUDO_DEBUG_INFO,
"%s: received RestartMessage for %s from %s", __func__, msg->log_id,
source);
Expand Down Expand Up @@ -642,6 +655,12 @@ handle_iobuf(int iofd, IoBuffer *iobuf, uint8_t *buf, size_t len,
debug_return_bool(false);
}

/* Check that message is valid. */
if (iobuf->delay == NULL) {
sudo_warnx(U_("%s: %s"), source, U_("invalid IoBuffer"));
closure->errstr = _("invalid IoBuffer");
debug_return_bool(false);
}
sudo_debug_printf(SUDO_DEBUG_INFO, "%s: received IoBuffer from %s",
source, __func__);

Expand Down Expand Up @@ -672,6 +691,12 @@ handle_winsize(ChangeWindowSize *msg, uint8_t *buf, size_t len,
debug_return_bool(false);
}

/* Check that message is valid. */
if (msg->delay == NULL) {
sudo_warnx(U_("%s: %s"), source, U_("invalid ChangeWindowSize"));
closure->errstr = _("invalid ChangeWindowSize");
debug_return_bool(false);
}
sudo_debug_printf(SUDO_DEBUG_INFO, "%s: received ChangeWindowSize from %s",
source, __func__);

Expand Down Expand Up @@ -702,6 +727,12 @@ handle_suspend(CommandSuspend *msg, uint8_t *buf, size_t len,
debug_return_bool(false);
}

/* Check that message is valid. */
if (msg->delay == NULL || msg->signal == NULL) {
sudo_warnx(U_("%s: %s"), source, U_("invalid CommandSuspend"));
closure->errstr = _("invalid CommandSuspend");
debug_return_bool(false);
}
sudo_debug_printf(SUDO_DEBUG_INFO, "%s: received CommandSuspend from %s",
source, __func__);

Expand Down Expand Up @@ -730,7 +761,7 @@ handle_client_hello(ClientHello *msg, uint8_t *buf, size_t len,
sudo_debug_printf(SUDO_DEBUG_INFO, "%s: received ClientHello",
__func__);
sudo_debug_printf(SUDO_DEBUG_INFO, "%s: client ID %s",
__func__, msg->client_id);
__func__, msg->client_id ? msg->client_id : "unknown");

debug_return_bool(true);
}
Expand Down
Loading

0 comments on commit e6f2ad0

Please sign in to comment.