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 277b55b416ed7a13406e9e17244cb54e527b6d60..4342183088e71daf12dc9a198ba678487671091e 100644 (file)
--- 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 20e744a02e379cd42af0db12a4c9f014e80e5226..4fbc7d0b38ceba66aced21bd47a020028b8d6474 100644 (file)
--- 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 9f88133bc597cb22d7e5a90ec615f1cba2ed05d7..0718a2e8cac7cb12a10f56b32dbc1886c4f59ac8 100644 (file)
--- 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();