ACK, comments inline are very minor.
On 09/23/2011 02:43 PM, Adam Stokes wrote:
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(a)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_-.";
This can be static const.
+ 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) {
You can just use g_strstr() (or actually just strchr() if you want) here.
+ 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++) {
Instead of 3, use DIMOF(invalid_keys).
+ TS_ASSERT((mh_sysconfig_set_configured(invalid_keys[i],
"OK")) == FALSE);
+ }
}
};
--
Russell Bryant