There is basically no limitation on how the key name is defined. With that said we should at least make sure the keys do not start with a '/' or '..'. This will eliminate accidental typos and also intentional ones.
Signed-off-by: Adam Stokes astokes@fedoraproject.org --- src/lib/sysconfig.c | 16 +++++++++++++++- src/tests/mh_api_sysconfig.h | 6 ++++++ 2 files changed, 21 insertions(+), 1 deletions(-)
diff --git a/src/lib/sysconfig.c b/src/lib/sysconfig.c index 0301734..92bbfd4 100644 --- a/src/lib/sysconfig.c +++ b/src/lib/sysconfig.c @@ -68,10 +68,24 @@ static gboolean set_key(const char *key, const char *contents) { char key_file[PATH_MAX]; + char valid_chars[] = "0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ_-."; + char sanitized_key[PATH_MAX];
if (mh_strlen_zero(key)) { mh_err("key cannot be empty"); - return -1; + return FALSE; + } + + if (g_str_has_prefix(key, ".")) { + mh_err("Invalid key filename %s", key); + return FALSE; + } + + mh_string_copy(sanitized_key, key, sizeof(sanitized_key)); + g_strcanon(sanitized_key, valid_chars, '!'); + if (g_strstr_len(sanitized_key, PATH_MAX, "!") != NULL) { + mh_err("Invalid key filename %s", sanitized_key); + return FALSE; }
if (!g_file_test(keys_dir_get(), G_FILE_TEST_IS_DIR) && diff --git a/src/tests/mh_api_sysconfig.h b/src/tests/mh_api_sysconfig.h index 0bab713..0088df5 100644 --- a/src/tests/mh_api_sysconfig.h +++ b/src/tests/mh_api_sysconfig.h @@ -29,6 +29,9 @@ class MhApiSysconfigSuite : public CxxTest::TestSuite void testIsConfigured(void) { const char key[] = "org.matahariproject.test.unittest"; // Unimportant key defined + const char *invalid_keys[3] = {"../etc/passwd", + "./../etc/passwd#", + "HAPPY#HAMMY,@"}; char *key_res;
mh_sysconfig_keys_dir_set("/tmp/matahari-sysconfig-keys/"); @@ -38,6 +41,9 @@ class MhApiSysconfigSuite : public CxxTest::TestSuite TS_ASSERT(!strcmp("OK", key_res));
free(key_res); + for (int i = 0; i < 3; i++) { + TS_ASSERT((mh_sysconfig_set_configured(invalid_keys[i], "OK")) == FALSE); + } } };