From: Tomasz Miąsko <tomasz.miasko@gmail.com>
Date: Wed, 2 May 2018 00:00:00 +0000 (+0000)
Subject: Avoid TOCTOU issue when deciding if config is valid
X-Git-Tag: v3.4.3~11
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c8590d9016445549405fc95995a9d89c9bd0f94b;p=thirdparty%2Fccache.git

Avoid TOCTOU issue when deciding if config is valid

Previously, a separate call to access had been used to distinguish
between I/O errors and invalid configuration file. This could lead to
spurious errors if configuration file have been created in-between call
to conf_read and access. Use errno to tell those two cases apart.

Closes #260.
---

diff --git a/src/ccache.c b/src/ccache.c
index 277b55b41..434218308 100644
--- a/src/ccache.c
+++ b/src/ccache.c
@@ -3124,7 +3124,7 @@ initialize(void)
 	} else {
 		secondary_config_path = format("%s/ccache.conf", TO_STRING(SYSCONFDIR));
 		if (!conf_read(conf, secondary_config_path, &errmsg)) {
-			if (access(secondary_config_path, R_OK) == 0) {
+			if (errno == 0) {
 				// We could read the file but it contained errors.
 				fatal("%s", errmsg);
 			}
@@ -3148,7 +3148,7 @@ initialize(void)
 
 	bool should_create_initial_config = false;
 	if (!conf_read(conf, primary_config_path, &errmsg)) {
-		if (access(primary_config_path, R_OK) == 0) {
+		if (errno == 0) {
 			// We could read the file but it contained errors.
 			fatal("%s", errmsg);
 		}
diff --git a/src/conf.c b/src/conf.c
index 20e744a02..4fbc7d0b3 100644
--- a/src/conf.c
+++ b/src/conf.c
@@ -381,6 +381,9 @@ conf_free(struct conf *conf)
 }
 
 // Note: The path pointer is stored in conf, so path must outlive conf.
+//
+// On failure, if an I/O error occured errno is set approriately, otherwise
+// errno is set to zero indicating that config itself was invalid.
 bool
 conf_read(struct conf *conf, const char *path, char **errmsg)
 {
@@ -411,6 +414,7 @@ conf_read(struct conf *conf, const char *path, char **errmsg)
 		if (!ok) {
 			*errmsg = format("%s:%u: %s", path, line_number, errmsg2);
 			free(errmsg2);
+			errno = 0;
 			result = false;
 			goto out;
 		}
diff --git a/unittest/test_conf.c b/unittest/test_conf.c
index 9f88133bc..0718a2e8c 100644
--- a/unittest/test_conf.c
+++ b/unittest/test_conf.c
@@ -183,6 +183,7 @@ TEST(conf_read_with_missing_equal_sign)
 	char *errmsg;
 	create_file("ccache.conf", "no equal sign");
 	CHECK(!conf_read(conf, "ccache.conf", &errmsg));
+	CHECK_INT_EQ(errno, 0);
 	CHECK_STR_EQ_FREE2("ccache.conf:1: missing equal sign",
 	                   errmsg);
 	conf_free(conf);
@@ -194,6 +195,7 @@ TEST(conf_read_with_bad_config_key)
 	char *errmsg;
 	create_file("ccache.conf", "# Comment\nfoo = bar");
 	CHECK(!conf_read(conf, "ccache.conf", &errmsg));
+	CHECK_INT_EQ(errno, 0);
 	CHECK_STR_EQ_FREE2("ccache.conf:2: unknown configuration option \"foo\"",
 	                   errmsg);
 	conf_free(conf);
@@ -206,11 +208,13 @@ TEST(conf_read_invalid_bool)
 
 	create_file("ccache.conf", "disable=");
 	CHECK(!conf_read(conf, "ccache.conf", &errmsg));
+	CHECK_INT_EQ(errno, 0);
 	CHECK_STR_EQ_FREE2("ccache.conf:1: not a boolean value: \"\"",
 	                   errmsg);
 
 	create_file("ccache.conf", "disable=foo");
 	CHECK(!conf_read(conf, "ccache.conf", &errmsg));
+	CHECK_INT_EQ(errno, 0);
 	CHECK_STR_EQ_FREE2("ccache.conf:1: not a boolean value: \"foo\"",
 	                   errmsg);
 	conf_free(conf);
@@ -222,6 +226,7 @@ TEST(conf_read_invalid_env_string)
 	char *errmsg;
 	create_file("ccache.conf", "base_dir = ${foo");
 	CHECK(!conf_read(conf, "ccache.conf", &errmsg));
+	CHECK_INT_EQ(errno, 0);
 	CHECK_STR_EQ_FREE2("ccache.conf:1: syntax error: missing '}' after \"foo\"",
 	                   errmsg);
 	// Other cases tested in test_util.c.
@@ -244,6 +249,7 @@ TEST(conf_read_invalid_size)
 	char *errmsg;
 	create_file("ccache.conf", "max_size = foo");
 	CHECK(!conf_read(conf, "ccache.conf", &errmsg));
+	CHECK_INT_EQ(errno, 0);
 	CHECK_STR_EQ_FREE2("ccache.conf:1: invalid size: \"foo\"",
 	                   errmsg);
 	// Other cases tested in test_util.c.
@@ -256,6 +262,7 @@ TEST(conf_read_invalid_sloppiness)
 	char *errmsg;
 	create_file("ccache.conf", "sloppiness = file_macro, foo");
 	CHECK(!conf_read(conf, "ccache.conf", &errmsg));
+	CHECK_INT_EQ(errno, 0);
 	CHECK_STR_EQ_FREE2("ccache.conf:1: unknown sloppiness: \"foo\"",
 	                   errmsg);
 	conf_free(conf);
@@ -268,6 +275,7 @@ TEST(conf_read_invalid_unsigned)
 
 	create_file("ccache.conf", "max_files =");
 	CHECK(!conf_read(conf, "ccache.conf", &errmsg));
+	CHECK_INT_EQ(errno, 0);
 	CHECK_STR_EQ_FREE2("ccache.conf:1: invalid unsigned integer: \"\"",
 	                   errmsg);
 
@@ -284,6 +292,14 @@ TEST(conf_read_invalid_unsigned)
 	conf_free(conf);
 }
 
+TEST(conf_read_missing_config_file)
+{
+	struct conf *conf = conf_create();
+	char *errmsg;
+	CHECK(!conf_read(conf, "ccache.conf", &errmsg));
+	CHECK_INT_EQ(errno, ENOENT);
+}
+
 TEST(verify_absolute_base_dir)
 {
 	struct conf *conf = conf_create();