Add missing NULL checks for mandatory fields in protobuf messages.

Also no longer reject an InfoMessage with an unknown value_case,
just log and ignore it.

logsrvd/iolog_writer.c

@@ -54,22 +54,28 @@
 
 #include "logsrvd.h"
 
-static inline bool
-has_numval(InfoMessage *info)
+static bool
+type_matches(InfoMessage *info, const char *source,
+    InfoMessage__ValueCase value_case)
 {
-    return info->value_case == INFO_MESSAGE__VALUE_NUMVAL;
-}
+    const void *val = info->u.strval;	/* same for strlistval */
+    debug_decl(type_matches, SUDO_DEBUG_UTIL);
 
-static inline bool
-has_strval(InfoMessage *info)
-{
-    return info->value_case == INFO_MESSAGE__VALUE_STRVAL;
-}
-
-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);
 }
 
 /*
@@ -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;
 	    }

logsrvd/logsrvd.c

@@ -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__);
 
@@ -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);
@@ -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__);
 
@@ -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__);
 
@@ -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__);
 
@@ -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);
 }

logsrvd/logsrvd_local.c

@@ -105,6 +105,11 @@ logsrvd_json_log_cb(struct json_container *json, void *v)
 		goto bad;
 	    break;
 	case INFO_MESSAGE__VALUE_STRVAL:
+	    if (info->u.strval == NULL) {
+		sudo_warnx(U_("%s: protocol error: NULL value found in %s"),
+		    "local", info->key);
+		break;
+	    }
 	    json_value.type = JSON_STRING;
 	    json_value.u.string = info->u.strval;
 	    if (!sudo_json_add_value(json, info->key, &json_value))
@@ -114,9 +119,19 @@ logsrvd_json_log_cb(struct json_container *json, void *v)
 	    InfoMessage__StringList *strlist = info->u.strlistval;
 	    size_t n;
 
+	    if (strlist == NULL) {
+		sudo_warnx(U_("%s: protocol error: NULL value found in %s"),
+		    "local", info->key);
+		break;
+	    }
 	    if (!sudo_json_open_array(json, info->key))
 		goto bad;
 	    for (n = 0; n < strlist->n_strings; n++) {
+		if (strlist->strings[n] == NULL) {
+		    sudo_warnx(U_("%s: protocol error: NULL value found in %s"),
+			"local", info->key);
+		    break;
+		}
 		json_value.type = JSON_STRING;
 		json_value.u.string = strlist->strings[n];
 		if (!sudo_json_add_value(json, NULL, &json_value))
@@ -127,9 +142,9 @@ logsrvd_json_log_cb(struct json_container *json, void *v)
 	    break;
 	}
 	default:
-	    sudo_warnx(U_("unexpected type_case value %d in %s from %s"),
+	    sudo_warnx(U_("unexpected value_case %d in %s from %s"),
 		info->value_case, "InfoMessage", "local");
-	    goto bad;
+	    break;
 	}
     }
     debug_return_bool(true);

plugins/sudoers/log_client.c

@@ -909,9 +909,9 @@ fmt_info_messages(struct client_closure *closure, struct eventlog *evlog,
     fill_str("submithost", evlog->submithost);
     /* TODO - submituid */
     fill_str("submituser", evlog->submituser);
-    if (evlog->ttyname != NULL) {
+//    if (evlog->ttyname != NULL) {
 	fill_str("ttyname", evlog->ttyname);
-    }
+ //   }
 
     /* Free unused structs. */
     while (info_msgs_size > n)