Add basic regress for JSON functions.

Fix a bug in escaped control character handling.
Roll back changes to buffer if sudo_json_add_value() fails.

MANIFEST

@@ -280,6 +280,7 @@ lib/util/regress/glob/files
 lib/util/regress/glob/globtest.c
 lib/util/regress/glob/globtest.in
 lib/util/regress/harness.in
+lib/util/regress/json/json_test.c
 lib/util/regress/mktemp/mktemp_test.c
 lib/util/regress/multiarch/multiarch_test.c
 lib/util/regress/open_parent_dir/open_parent_dir_test.c

include/sudo_json.h

@@ -63,10 +63,12 @@ struct json_container {
     bool minimal;
     bool memfatal;
     bool need_comma;
+    bool quiet;
 };
 
 sudo_dso_public bool sudo_json_init_v1(struct json_container *jsonc, int indent, bool minimal, bool memfatal);
-#define sudo_json_init(_a, _b, _c, _d) sudo_json_init_v1((_a), (_b), (_c), (_d))
+sudo_dso_public bool sudo_json_init_v2(struct json_container *jsonc, int indent, bool minimal, bool memfatal, bool quiet);
+#define sudo_json_init(_a, _b, _c, _d, _e) sudo_json_init_v2((_a), (_b), (_c), (_d), (_e))
 
 sudo_dso_public void sudo_json_free_v1(struct json_container *jsonc);
 #define sudo_json_free(_a) sudo_json_free_v1((_a))

lib/eventlog/eventlog.c

@@ -880,7 +880,7 @@ format_json(int event_type, struct eventlog_args *args,
 	debug_return_str(NULL);
     }
 
-    if (!sudo_json_init(&jsonc, 4, compact, false))
+    if (!sudo_json_init(&jsonc, 4, compact, false, false))
 	goto bad;
     if (!sudo_json_open_object(&jsonc, type_str))
 	goto bad;

lib/iolog/iolog_loginfo.c

@@ -160,7 +160,7 @@ iolog_write_info_file_json(int dfd, struct eventlog *evlog)
     int fd = -1;
     debug_decl(iolog_write_info_file_json, SUDO_DEBUG_UTIL);
 
-    if (!sudo_json_init(&jsonc, 4, false, false))
+    if (!sudo_json_init(&jsonc, 4, false, false, false))
 	debug_return_bool(false);
 
     /* Timestamp */

lib/iolog/regress/iolog_json/check_iolog_json.c

@@ -202,7 +202,7 @@ main(int argc, char *argv[])
 
 	ntests++;
 
-	if (!sudo_json_init(&jsonc, 4, false, true)) {
+	if (!sudo_json_init(&jsonc, 4, false, true, true)) {
 	    errors++;
 	    continue;
 	}

lib/util/Makefile.in

@@ -111,10 +111,11 @@ PVS_IGNORE = 'V707,V011,V002,V536'
 PVS_LOG_OPTS = -a 'GA:1,2' -e -t errorfile -d $(PVS_IGNORE)
 
 # Regression tests
-TEST_PROGS = conf_test hltq_test parseln_test progname_test parse_gids_test \
+TEST_PROGS = conf_test getgids getgrouplist_test hltq_test json_test \
-	     getgids getgrouplist_test multiarch_test open_parent_dir_test \
+             multiarch_test open_parent_dir_test parse_gids_test parseln_test \
-	     strsplit_test strtobool_test strtoid_test strtomode_test \
+	     progname_test strsplit_test strtobool_test strtoid_test \
-	     strtonum_test uuid_test @COMPAT_TEST_PROGS@
+	     strtomode_test strtonum_test uuid_test @COMPAT_TEST_PROGS@
+
 TEST_LIBS = @LIBS@
 TEST_LDFLAGS = @LDFLAGS@
 TEST_VERBOSE =
@@ -164,14 +165,16 @@ CLOSEFROM_TEST_OBJS = closefrom_test.lo closefrom.lo
 
 CONF_TEST_OBJS = conf_test.lo sudo_conf.lo
 
-HLTQ_TEST_OBJS = hltq_test.lo
-
 FNM_TEST_OBJS = fnm_test.lo fnmatch.lo
 
 GLOBTEST_OBJS = globtest.lo glob.lo
 
 GETDELIM_TEST_OBJS = getdelim_test.lo getdelim.lo
 
+HLTQ_TEST_OBJS = hltq_test.lo
+
+JSON_TEST_OBJS = json_test.lo json.lo
+
 MULTIARCH_TEST_OBJS = multiarch_test.lo multiarch.lo
 
 OPEN_PARENT_DIR_TEST_OBJS = open_parent_dir_test.lo mkdir_parents.lo
@@ -284,6 +287,9 @@ getdelim_test: $(GETDELIM_TEST_OBJS) libsudo_util.la
 hltq_test: $(HLTQ_TEST_OBJS) libsudo_util.la
 	$(LIBTOOL) $(LTFLAGS) --mode=link $(CC) -o $@ $(HLTQ_TEST_OBJS) libsudo_util.la $(ASAN_LDFLAGS) $(PIE_LDFLAGS) $(HARDENING_LDFLAGS) $(TEST_LDFLAGS) $(TEST_LIBS)
 
+json_test: $(JSON_TEST_OBJS) libsudo_util.la
+	$(LIBTOOL) $(LTFLAGS) --mode=link $(CC) -o $@ $(JSON_TEST_OBJS) libsudo_util.la $(ASAN_LDFLAGS) $(PIE_LDFLAGS) $(HARDENING_LDFLAGS) $(TEST_LDFLAGS) $(TEST_LIBS)
+
 mktemp_test: $(MKTEMP_TEST_OBJS) libsudo_util.la
 	$(LIBTOOL) $(LTFLAGS) --mode=link $(CC) -o $@ $(MKTEMP_TEST_OBJS) libsudo_util.la $(ASAN_LDFLAGS) $(PIE_LDFLAGS) $(HARDENING_LDFLAGS) $(TEST_LDFLAGS) $(TEST_LIBS)
 
@@ -467,6 +473,7 @@ check: $(TEST_PROGS) check-fuzzer
 	    ./strtonum_test || rval=`expr $$rval + $$?`; \
 	    ./uuid_test || rval=`expr $$rval + $$?`; \
 	    ./hltq_test || rval=`expr $$rval + $$?`; \
+	    ./json_test || rval=`expr $$rval + $$?`; \
 	    ./progname_test || rval=`expr $$rval + $$?`; \
 	    rm -f ./progname_test2; ln -s ./progname_test ./progname_test2; \
 	    ./progname_test2 || rval=`expr $$rval + $$?`; \
@@ -975,6 +982,18 @@ json.i: $(srcdir)/json.c $(incdir)/compat/stdbool.h $(incdir)/sudo_compat.h \
 	$(CC) -E -o $@ $(CPPFLAGS) $<
 json.plog: json.i
 	rm -f $@; pvs-studio --cfg $(PVS_CFG) --sourcetree-root $(top_srcdir) --skip-cl-exe yes --source-file $(srcdir)/json.c --i-file $< --output-file $@
+json_test.lo: $(srcdir)/regress/json/json_test.c $(incdir)/compat/stdbool.h \
+              $(incdir)/sudo_compat.h $(incdir)/sudo_fatal.h \
+              $(incdir)/sudo_plugin.h $(incdir)/sudo_util.h \
+              $(top_builddir)/config.h
+	$(LIBTOOL) $(LTFLAGS) --mode=compile $(CC) -c -o $@ $(CPPFLAGS) $(CFLAGS) $(ASAN_CFLAGS) $(PIE_CFLAGS) $(HARDENING_CFLAGS) $(srcdir)/regress/json/json_test.c
+json_test.i: $(srcdir)/regress/json/json_test.c $(incdir)/compat/stdbool.h \
+              $(incdir)/sudo_compat.h $(incdir)/sudo_fatal.h \
+              $(incdir)/sudo_plugin.h $(incdir)/sudo_util.h \
+              $(top_builddir)/config.h
+	$(CC) -E -o $@ $(CPPFLAGS) $<
+json_test.plog: json_test.i
+	rm -f $@; pvs-studio --cfg $(PVS_CFG) --sourcetree-root $(top_srcdir) --skip-cl-exe yes --source-file $(srcdir)/regress/json/json_test.c --i-file $< --output-file $@
 key_val.lo: $(srcdir)/key_val.c $(incdir)/compat/stdbool.h \
             $(incdir)/sudo_compat.h $(incdir)/sudo_debug.h \
             $(incdir)/sudo_queue.h $(incdir)/sudo_util.h \

lib/util/json.c

@@ -164,7 +164,7 @@ json_append_string(struct json_container *jsonc, const char *str)
 		*cp++ = '0';
 		*cp++ = '0';
 		*cp++ = hex[ch >> 4];
-		ch &= 0x0f;
+		ch = hex[ch & 0x0f];
 	    }
 	    break;
 	}
@@ -180,8 +180,8 @@ json_append_string(struct json_container *jsonc, const char *str)
 }
 
 bool
-sudo_json_init_v1(struct json_container *jsonc, int indent, bool minimal,
+sudo_json_init_v2(struct json_container *jsonc, int indent, bool minimal,
-    bool memfatal)
+    bool memfatal, bool quiet)
 {
     debug_decl(sudo_json_init, SUDO_DEBUG_UTIL);
 
@@ -190,6 +190,7 @@ sudo_json_init_v1(struct json_container *jsonc, int indent, bool minimal,
     jsonc->indent_increment = indent;
     jsonc->minimal = minimal;
     jsonc->memfatal = memfatal;
+    jsonc->quiet = quiet;
     jsonc->buf = malloc(64 * 1024);
     if (jsonc->buf == NULL) {
 	if (jsonc->memfatal) {
@@ -206,6 +207,13 @@ sudo_json_init_v1(struct json_container *jsonc, int indent, bool minimal,
     debug_return_bool(true);
 }
 
+bool
+sudo_json_init_v1(struct json_container *jsonc, int indent, bool minimal,
+    bool memfatal)
+{
+    return sudo_json_init_v2(jsonc, indent, minimal, memfatal, false);
+}
+
 void
 sudo_json_free_v1(struct json_container *jsonc)
 {
@@ -309,69 +317,83 @@ static bool
 sudo_json_add_value_int(struct json_container *jsonc, const char *name,
     struct json_value *value, bool as_object)
 {
+    struct json_container saved_container = *jsonc;
     char numbuf[(((sizeof(long long) * 8) + 2) / 3) + 2];
     debug_decl(sudo_json_add_value, SUDO_DEBUG_UTIL);
 
     /* Add comma if we are continuing an object/array. */
     if (jsonc->need_comma) {
 	if (!json_append_buf(jsonc, ","))
-	    debug_return_bool(false);
+	    goto bad;
     }
     if (!json_new_line(jsonc))
-	debug_return_bool(false);
+	goto bad;
     jsonc->need_comma = true;
 
     if (as_object) {
 	if (!json_append_buf(jsonc, jsonc->minimal ? "{" : "{ "))
-	    debug_return_bool(false);
+	    goto bad;
     }
 
     /* name */
     if (name != NULL) {
 	if (!json_append_string(jsonc, name))
-	    debug_return_bool(false);
+	    goto bad;
 	if (!json_append_buf(jsonc, jsonc->minimal ? ":" : ": "))
-	    debug_return_bool(false);
+	    goto bad;
     }
 
     /* value */
     switch (value->type) {
     case JSON_STRING:
 	if (!json_append_string(jsonc, value->u.string))
-	    debug_return_bool(false);
+	    goto bad;
 	break;
     case JSON_ID:
 	snprintf(numbuf, sizeof(numbuf), "%u", (unsigned int)value->u.id);
 	if (!json_append_buf(jsonc, numbuf))
-	    debug_return_bool(false);
+	    goto bad;
 	break;
     case JSON_NUMBER:
 	snprintf(numbuf, sizeof(numbuf), "%lld", value->u.number);
 	if (!json_append_buf(jsonc, numbuf))
-	    debug_return_bool(false);
+	    goto bad;
 	break;
     case JSON_NULL:
 	if (!json_append_buf(jsonc, "null"))
-	    debug_return_bool(false);
+	    goto bad;
 	break;
     case JSON_BOOL:
 	if (!json_append_buf(jsonc, value->u.boolean ? "true" : "false"))
-	    debug_return_bool(false);
+	    goto bad;
 	break;
     case JSON_ARRAY:
-	sudo_fatalx("internal error: can't print JSON_ARRAY");
+	if (!jsonc->quiet)
-	break;
+	    sudo_warnx("internal error: add JSON_ARRAY as a value");
+	goto bad;
     case JSON_OBJECT:
-	sudo_fatalx("internal error: can't print JSON_OBJECT");
+	if (!jsonc->quiet)
-	break;
+	    sudo_warnx("internal error: add JSON_OBJECT as a value");
+	goto bad;
+    default:
+	if (!jsonc->quiet)
+	    sudo_warnx("internal error: unknown JSON type %d", value->type);
+	goto bad;
     }
 
     if (as_object) {
 	if (!json_append_buf(jsonc, jsonc->minimal ? "}" : " }"))
-	    debug_return_bool(false);
+	    goto bad;
     }
 
     debug_return_bool(true);
+bad:
+    /* Restore container but handle reallocation of buf. */
+    saved_container.buf = jsonc->buf;
+    saved_container.bufsize = jsonc->bufsize;
+    *jsonc = saved_container;
+    jsonc->buf[jsonc->buflen] = '\0';
+    debug_return_bool(false);
 }
 
 bool

lib/util/regress/json/json_test.c

@@ -0,0 +1,233 @@
+/*
+ * SPDX-License-Identifier: ISC
+ *
+ * Copyright (c) 2022 Todd C. Miller <Todd.Miller@sudo.ws>
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include <config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <errno.h>
+#include <limits.h>
+#include <string.h>
+#include <unistd.h>
+
+#define SUDO_ERROR_WRAP 0
+
+#include "sudo_compat.h"
+#include "sudo_json.h"
+#include "sudo_util.h"
+#include "sudo_fatal.h"
+
+sudo_dso_public int main(int argc, char *argv[]);
+
+/* Expected JSON output */
+const char outbuf[] = "\n"
+    "    \"test1\": {\n"
+    "        \"string1\": \"test\\\\\\b\\f\\n\\r\\t string1\",\n"
+    "        \"id1\": 4294967295,\n"
+    "        \"number1\": -1,\n"
+    "        \"bool1\": true,\n"
+    "        \"bool2\": false,\n"
+    "        \"null1\": null,\n"
+    "        \"array1\": [\n"
+    "            \"string2\": \"test\\f\\u0011string2\",\n"
+    "            \"number2\": -9223372036854775808,\n"
+    "            \"number3\": 9223372036854775807\n"
+    "        ]\n"
+    "    }";
+
+/*
+ * Simple tests for sudo json functions()
+ */
+int
+main(int argc, char *argv[])
+{
+    struct json_container jsonc;
+    struct json_value value;
+    int ch, errors = 0, ntests = 0;
+
+    initprogname(argc > 0 ? argv[0] : "json_test");
+
+    while ((ch = getopt(argc, argv, "v")) != -1) {
+	switch (ch) {
+	case 'v':
+	    /* ignore */
+	    break;
+	default:
+	    fprintf(stderr, "usage: %s [-v]\n", getprogname());
+	    return EXIT_FAILURE;
+	}
+    }
+    argc -= optind;
+    argv += optind;
+
+    ntests++;
+    if (!sudo_json_init(&jsonc, 4, false, true, true)) {
+	sudo_warnx("unable to initialize json");
+	errors++;
+	goto done;
+    }
+
+    /* Open main JSON object. */
+    ntests++;
+    if (!sudo_json_open_object(&jsonc, "test1")) {
+	sudo_warnx("unable to open json object");
+	errors++;
+	goto done;
+    }
+
+    /* Verify invalid value is detected. */
+    value.type = -1;
+    value.u.string = NULL;
+    ntests++;
+    if (sudo_json_add_value(&jsonc, "bogus1", &value)) {
+	/* should have failed, not a fatal error */
+	sudo_warnx("should not be able to add bogus type value");
+	errors++;
+    }
+
+    /* Verify that adding an array is not allowed. */
+    value.type = JSON_ARRAY;
+    value.u.string = NULL;
+    ntests++;
+    if (sudo_json_add_value(&jsonc, "bogus2", &value)) {
+	/* should have failed, not a fatal error */
+	sudo_warnx("should not be able to add array type value");
+	errors++;
+    }
+
+    /* Verify that adding an object is not allowed. */
+    value.type = JSON_OBJECT;
+    value.u.string = NULL;
+    ntests++;
+    if (sudo_json_add_value(&jsonc, "bogus3", &value)) {
+	/* should have failed, not a fatal error */
+	sudo_warnx("should not be able to add object type value");
+	errors++;
+    }
+
+    value.type = JSON_STRING;
+    value.u.string = "test\\\b\f\n\r\t string1";
+    ntests++;
+    if (!sudo_json_add_value(&jsonc, "string1", &value)) {
+	/* not a fatal error */
+	sudo_warnx("unable to add string value (string1)");
+	errors++;
+    }
+
+    value.type = JSON_ID;
+    value.u.id = 0xffffffff;
+    ntests++;
+    if (!sudo_json_add_value(&jsonc, "id1", &value)) {
+	/* not a fatal error */
+	sudo_warnx("unable to add ID value (0xffffffff)");
+	errors++;
+    }
+
+    value.type = JSON_NUMBER;
+    value.u.number = -1;
+    ntests++;
+    if (!sudo_json_add_value(&jsonc, "number1", &value)) {
+	/* not a fatal error */
+	sudo_warnx("unable to add number value (-1)");
+	errors++;
+    }
+
+    value.type = JSON_BOOL;
+    value.u.boolean = true;
+    ntests++;
+    if (!sudo_json_add_value(&jsonc, "bool1", &value)) {
+	/* not a fatal error */
+	sudo_warnx("unable to add bool value (true)");
+	errors++;
+    }
+    value.u.boolean = false;
+    ntests++;
+    if (!sudo_json_add_value(&jsonc, "bool2", &value)) {
+	/* not a fatal error */
+	sudo_warnx("unable to add bool value (false)");
+	errors++;
+    }
+
+    value.type = JSON_NULL;
+    ntests++;
+    if (!sudo_json_add_value(&jsonc, "null1", &value)) {
+	/* not a fatal error */
+	sudo_warnx("unable to add null value");
+	errors++;
+    }
+
+    /* Open JSON array. */
+    ntests++;
+    if (!sudo_json_open_array(&jsonc, "array1")) {
+	sudo_warnx("unable to open json array");
+	errors++;
+	goto done;
+    }
+
+    value.type = JSON_STRING;
+    value.u.string = "test\x0c\x11string2";
+    ntests++;
+    if (!sudo_json_add_value(&jsonc, "string2", &value)) {
+	/* not a fatal error */
+	sudo_warnx("unable to add string value (string2)");
+	errors++;
+    }
+
+    value.type = JSON_NUMBER;
+    value.u.number = LLONG_MIN;
+    ntests++;
+    if (!sudo_json_add_value(&jsonc, "number2", &value)) {
+	/* not a fatal error */
+	sudo_warnx("unable to add number value (LLONG_MIN)");
+	errors++;
+    }
+    value.u.number = LLONG_MAX;
+    ntests++;
+    if (!sudo_json_add_value(&jsonc, "number3", &value)) {
+	/* not a fatal error */
+	sudo_warnx("unable to add number value (LLONG_MAX)");
+	errors++;
+    }
+
+    /* Close JSON array. */
+    if (!sudo_json_close_array(&jsonc)) {
+	sudo_warnx("unable to close json array");
+	errors++;
+	goto done;
+    }
+
+    /* Close main JSON object. */
+    if (!sudo_json_close_object(&jsonc)) {
+	sudo_warnx("unable to close json object");
+	errors++;
+	goto done;
+    }
+
+    if (strcmp(outbuf, jsonc.buf) != 0) {
+	fprintf(stderr, "Expected:\n%s\n", outbuf);
+	fprintf(stderr, "Received:\n%s\n", jsonc.buf);
+    }
+
+done:
+    if (ntests != 0) {
+	printf("%s: %d tests run, %d errors, %d%% success rate\n",
+	    getprogname(), ntests, errors, (ntests - errors) * 100 / ntests);
+    }
+
+    return errors;
+}

lib/util/util.exp.in

@@ -96,6 +96,7 @@ sudo_json_free_v1
 sudo_json_get_buf_v1
 sudo_json_get_len_v1
 sudo_json_init_v1
+sudo_json_init_v2
 sudo_json_open_array_v1
 sudo_json_open_object_v1
 sudo_lbuf_append_quoted_v1

logsrvd/logsrvd_local.c

@@ -312,7 +312,7 @@ store_exit_info_json(int dfd, struct eventlog *evlog)
     off_t pos;
     debug_decl(store_exit_info_json, SUDO_DEBUG_UTIL);
 
-    if (!sudo_json_init(&jsonc, 4, false, false))
+    if (!sudo_json_init(&jsonc, 4, false, false, false))
         goto done;
 
     fd = iolog_openat(dfd, "log.json", O_RDWR);

plugins/audit_json/audit_json.c

@@ -437,7 +437,7 @@ audit_write_exit_record(int exit_status, int error)
 	goto done;
     }
 
-    if (!sudo_json_init(&jsonc, 4, false, false))
+    if (!sudo_json_init(&jsonc, 4, false, false, false))
 	goto oom;
     if (!sudo_json_open_object(&jsonc, "exit"))
 	goto oom;
@@ -522,7 +522,7 @@ audit_write_record(const char *audit_str, const char *plugin_name,
 	goto done;
     }
 
-    if (!sudo_json_init(&jsonc, 4, false, false))
+    if (!sudo_json_init(&jsonc, 4, false, false, false))
 	goto oom;
     if (!sudo_json_open_object(&jsonc, audit_str))
 	goto oom;

plugins/sudoers/cvtsudoers_json.c

@@ -899,7 +899,7 @@ convert_sudoers_json(struct sudoers_parse_tree *parse_tree,
     }
 
     /* 4 space indent, non-compact, exit on memory allocation failure. */
-    sudo_json_init(&jsonc, 4, false, true);
+    sudo_json_init(&jsonc, 4, false, true, false);
 
     /* Dump Defaults in JSON format. */
     if (!ISSET(conf->suppress, SUPPRESS_DEFAULTS)) {