From 90d94c76156677bb3afe2d7b5dc6d25262d0eecb Mon Sep 17 00:00:00 2001 From: fedir Date: Mon, 19 May 2025 21:18:19 +0200 Subject: [PATCH] Fixed SQL injection Fixed an emabarassignly obvious SQL injection bug by throwing `sqlite3_exec` away. --- src/perm_permissions_table.c | 57 ++++++++++++++---------------------- 1 file changed, 22 insertions(+), 35 deletions(-) diff --git a/src/perm_permissions_table.c b/src/perm_permissions_table.c index 0759319..e1bcd98 100644 --- a/src/perm_permissions_table.c +++ b/src/perm_permissions_table.c @@ -207,42 +207,29 @@ void destroy_perm_permissions_table(void) { sqlite3_close(perm_database); } */ access_t check_perm_access_noparent(const char *filename, struct process_info pi) { + access_t ret = NDEF; + sqlite3_stmt *stmt = NULL; + const char *sql = "SELECT mode FROM permissions WHERE executable = ?1 " + "AND (( ?2 LIKE CONCAT(filename, \'%\') AND filename " + "GLOB \'*/\') OR filename = ?2 );"; + sqlite3_prepare_v2(perm_database, sql, -1, &stmt, NULL); + sqlite3_bind_text(stmt, 1, pi.name, -1, SQLITE_STATIC); + sqlite3_bind_text(stmt, 2, filename, -1, SQLITE_STATIC); - char *query = NULL; - int ret = asprintf(&query, - "SELECT * FROM %s WHERE executable = \'%s\' " - "AND ((\'%s\' LIKE CONCAT(filename, \'%%\') AND filename " - "GLOB \'*/\') OR filename = \'%s\');", - table_name, pi.name, filename, filename); - fprintf(stderr, "query: %s\n", query); + int step_ret = sqlite3_step(stmt); + if (step_ret == SQLITE_ROW) { + int mode_col = sqlite3_column_int(stmt, 0); + if (mode_col) { + ret = ALLOW; + } else { + ret = DENY; + } + } else { + fprintf(stderr, "SQLite error: %s\n", sqlite3_errstr(step_ret)); + } + sqlite3_finalize(stmt); - if (ret < 0) { - // If asprintf fails, the contents of query are undefined (see man - // asprintf). That does not explicitly rule out that query will be a valid - // pointer. But the risk of freeing a non-allocated pointer is too much to - // justify preparing for this. - fprintf(stderr, "Could not create query on access check"); - perror(""); - return NDEF; - } - - char *sqlite_error = NULL; - int flag = 0; - ret = sqlite3_exec(perm_database, query, set_flag, &flag, &sqlite_error); - free((void *)query); - if (ret != SQLITE_OK) { - fprintf(stderr, "SQLite returned an error: %s\n", sqlite_error); - sqlite3_free(sqlite_error); - return NDEF; - } - - if (flag == 1) { - return ALLOW; - } - if (flag == -1) { - return DENY; - } - return NDEF; + return ret; } /** @@ -310,7 +297,7 @@ int set_perm_access(const char *filename, struct process_info pi, // asprintf). That does not explicitly rule out that query will be a valid // pointer. But the risk of freeing a non-allocated pointer is too much to // justify preparing for this. - fprintf(stderr, "Could not create query on rule insertion"); + fprintf(stderr, "Could not create query on rule insertion\n"); perror(""); return 1; }