From f50fed6df8d617bbfe7eecc95d58b8dea6acfb35 Mon Sep 17 00:00:00 2001
From: schneider <schneider@blinkenlichts.net>
Date: Tue, 7 Apr 2020 00:50:07 +0200
Subject: [PATCH] change(config): Make the config interface more robust
---
epicardium/modules/config.c | 72 ++++++++++++++++++++++++++--------
pycardium/modules/sys_config.c | 17 ++++++--
2 files changed, 69 insertions(+), 20 deletions(-)
diff --git a/epicardium/modules/config.c b/epicardium/modules/config.c
index 6874318a1..2e5ca6f2f 100644
--- a/epicardium/modules/config.c
+++ b/epicardium/modules/config.c
@@ -127,7 +127,7 @@ static void add_config_pair(
slot->value_offset = value_offset;
}
-static char *trim(char *str)
+static void trim(char *str)
{
char *start = str;
while (*start && !isgraph((int)*start))
@@ -139,7 +139,8 @@ static char *trim(char *str)
end--;
end[1] = 0;
}
- return start;
+
+ memmove(str, start, strlen(start) + 1);
}
// parses one line of the config file
@@ -147,7 +148,7 @@ static void parse_line(char *line, int line_number, size_t line_offset)
{
char *line_start = line;
- line = trim(line);
+ trim(line);
//printf(line);
if (*line == '#') {
@@ -168,13 +169,17 @@ static void parse_line(char *line, int line_number, size_t line_offset)
}
*eq = 0;
- char *key = trim(line);
+ char *key = line;
+ trim(key);
+
if (*key == '\0') {
LOG_WARN("card10.cfg", "line %d: empty key", line_number);
return;
}
- char *value = trim(eq + 1);
+ char *value = eq + 1;
+ trim(value);
+
if (*value == '\0') {
LOG_WARN(
"card10.cfg",
@@ -326,7 +331,7 @@ int epic_config_get_string(const char *key, char *buf, size_t buf_len)
}
char *end = buf;
- while (!iscntrl(*end))
+ while (*end && !iscntrl(*end))
end++;
*end = 0;
trim(buf);
@@ -384,16 +389,45 @@ bool config_get_boolean_with_default(const char *key, bool default_value)
}
// TODO: don't allow things like "execute_elf" to be set
-int epic_config_set_string(const char *key, const char *value)
+int epic_config_set_string(const char *key, const char *value_in)
{
- config_slot *slot = find_config_slot(key);
- bool present = slot && slot->value_offset;
- int ret = 0;
+ char value[MAX_LINE_LENGTH + 1];
+
+ if (strlen(key) > MAX_LINE_LENGTH) {
+ return -EINVAL;
+ }
+
+ /* TODO: Change interface of trim to take the buffer and size directly */
+ if (strlen(value_in) > MAX_LINE_LENGTH) {
+ return -EINVAL;
+ }
+
+ strcpy(value, value_in);
+ trim(value);
if (snprintf(NULL, 0, "\n%s = %s\n", key, value) > MAX_LINE_LENGTH) {
return -EINVAL;
}
+ /* Check if key is sane. No control characters, spaces, equal signs or pounds allowed */
+ for (size_t i = 0; i < strlen(key); i++) {
+ char c = key[i];
+ if (!isgraph(c) || c == '=' || c == '#') {
+ return -EINVAL;
+ }
+ }
+ /* Check if value is sane. No control characters allowed */
+ for (size_t i = 0; i < strlen(value); i++) {
+ char c = value[i];
+ if (!isprint(c)) {
+ return -EINVAL;
+ }
+ }
+
+ config_slot *slot = find_config_slot(key);
+ bool present = slot && slot->value_offset;
+ int ret = 0;
+
if (!present) {
/* Easy case: We simply add the new option at the
* end of the file. */
@@ -467,20 +501,24 @@ int epic_config_set_string(const char *key, const char *value)
* Solution: Copy parts of the file, insert new value, copy
* rest, rename.
*/
- char buf[128];
+ char buf[MAX_LINE_LENGTH + 1];
int fd1 = -1;
int fd2 = -1;
ret = epic_config_get_string(key, buf, sizeof(buf));
- if (ret < 0) {
- LOG_DEBUG(
- "card10.cfg",
- "could not read old value (%d)",
- ret
- );
+ size_t nread = read_config_offset(
+ slot->value_offset, buf, sizeof(buf)
+ );
+ if (nread == 0) {
+ LOG_DEBUG("card10.cfg", "could not read old value", );
goto out;
}
+ char *end = buf;
+ while (*end && (!iscntrl(*end) || isblank(*end)))
+ end++;
+ *end = 0;
+
int old_len = strlen(buf);
fd1 = epic_file_open("card10.cfg", "r");
diff --git a/pycardium/modules/sys_config.c b/pycardium/modules/sys_config.c
index a46103279..a811a19a8 100644
--- a/pycardium/modules/sys_config.c
+++ b/pycardium/modules/sys_config.c
@@ -8,8 +8,9 @@
#include <stdbool.h>
-static void extract_string(mp_obj_t obj, char *buf, size_t buf_len, const char *error_message)
-{
+static void extract_string(
+ mp_obj_t obj, char *buf, size_t buf_len, const char *error_message
+) {
size_t len;
const char *str_ptr = mp_obj_str_get_data(obj, &len);
/*
@@ -25,10 +26,20 @@ static void extract_string(mp_obj_t obj, char *buf, size_t buf_len, const char *
static mp_obj_t mp_config_set_string(mp_obj_t key_in, mp_obj_t value_in)
{
+ const char *const forbidden_key = "execute_elf";
+
char key_str[128], value_str[128];
extract_string(key_in, key_str, sizeof(key_str), "key too long");
- extract_string(value_in, value_str, sizeof(value_str), "value too long");
+ extract_string(
+ value_in, value_str, sizeof(value_str), "value too long"
+ );
+
+ if (strstr(key_str, forbidden_key) != NULL ||
+ strstr(value_str, forbidden_key) != NULL) {
+ /* A Permission Error might be nice but is not available in MP */
+ mp_raise_ValueError("Not allowed");
+ }
int status = epic_config_set_string(key_str, value_str);
if (status < 0) {
--
GitLab