mirror of
https://github.com/coturn/coturn.git
synced 2026-06-11 09:44:33 +00:00
* Parameterize SQLite user-DB driver queries (eliminate SQL injection class) The SQLite driver built every statement with snprintf, interpolating caller-supplied values directly into the SQL text before sqlite3_prepare. That left every value-carrying query injectable (the admin-panel delete paths fixed in GHSA-v8hj-2xx7-xmp5 were the reachable instances; this removes the underlying class for this backend). Add a sqlite_prepare_bind() helper that prepares a statement using '?' placeholders and binds the values as text parameters, and route all value-carrying statements through it: users, secrets, origins, realm options, oauth keys, permission IPs, and admin users. Values never enter the SQL text again. The only remaining interpolation is the peer-ip table name (allowed/denied), which cannot be a bound parameter and is already validated against that whitelist by the caller; its realm/ip values are now bound. Static, parameter-free statements are unchanged. oauth timestamp/lifetime were previously rendered as numeric literals; they are now formatted to decimal text and bound. The oauth_key columns have integer affinity, so SQLite stores the bound text as the same integer and reads it back identically (verified). Add tests/test_sqlite_dbd.c: an interface test that drives the driver's public vtable against a throwaway database, covering every converted entry point plus a SQL-injection regression test. Run against the old string-interpolated driver the seven behavioral tests pass unchanged (parity) while the injection test fails (a boolean payload in del_user deletes a whole realm's users); against the new driver all eight pass. The driver is compiled in isolation via a small support/stub layer (tests/test_sqlite_support.*) so the suite needs no running server. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Parameterize PostgreSQL user-DB driver queries (eliminate SQL injection class) Like the SQLite driver, dbd_pgsql.c built every statement with snprintf, interpolating caller-supplied values into the SQL text and running it via PQexec -- which on PostgreSQL also permits stacked queries, the highest- impact instance of GHSA-v8hj-2xx7-xmp5. Add a pq_exec_params() helper around PQexecParams and route all value-carrying statements through it with $1,$2,... placeholders and the values passed as out-of-band text parameters: users, secrets, origins, realm options, oauth keys, permission IPs, and admin users. Values never enter the SQL text. The only remaining interpolation is the peer-ip table name (allowed/denied), which cannot be a bound parameter and is already whitelisted by the caller; its realm/ip values are now bound. Static, parameter-free SELECTs keep using PQexec. oauth timestamp/lifetime and the realm-option value were numeric literals; they are now formatted to decimal text and bound (PostgreSQL casts them to the column's integer type, preserving behavior). Add tests/test_pgsql_dbd.c with a link-seam libpq mock (test_pgsql_stub.c) that captures each emitted command, whether it was parameterized, and the bound parameter values -- so the driver is unit-tested with no server. The tests assert every operation emits a parameterized query with caller values bound out-of-band, and test_sql_injection_neutralized asserts a boolean payload travels as a bound value, never as SQL text. Run against the old driver all of these fail (it calls PQexec with values interpolated and binds nothing); against the new driver all pass. Relay-side stubs are reused from test_sqlite_support.c. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Parameterize MySQL user-DB driver with prepared statements (eliminate SQL injection class) dbd_mysql.c built every statement with snprintf and ran it via mysql_query(), interpolating caller-supplied values into the SQL text -- the same SQL injection class fixed for SQLite and PostgreSQL, now closed for the MySQL backend. Convert the whole driver to the prepared-statement API. Two helpers carry the risk: - my_exec() prepares an INSERT/UPDATE/DELETE, binds text params, executes; - my_query_rows() prepares a SELECT, binds text params and text result columns, and invokes a per-row callback (replacing mysql_query + mysql_store_result + mysql_fetch_row). Every value-carrying statement now uses '?' placeholders with values bound out-of-band; the read paths get small row callbacks. The only remaining interpolation is the peer-ip table name (allowed/denied), which cannot be a bound parameter and is already whitelisted by the caller; its realm/ip values are bound. oauth timestamp/lifetime and the realm-option value are formatted to decimal text and bound (MySQL casts to the column's integer type). Add tests/test_mysql_dbd.c with a link-seam libmysqlclient mock (test_mysql_stub.c) that captures the prepared SQL and bound parameters, and can feed one canned result row so the read paths' bind-result/fetch handling is exercised (get_user_key, get_oauth_key, get_admin_user, get_auth_secrets all round-trip). The mock also implements the classic mysql_query/store_result API so the suite links and runs against the old driver: there every test fails ("got mysql_query()") while all pass against the new one, and test_sql_injection_neutralized asserts a boolean payload travels as a bound value. Relay-side stubs are reused from test_sqlite_support.c (with ur_string_map_free added there). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
143 lines
4.0 KiB
C
143 lines
4.0 KiB
C
/*
|
|
* SPDX-License-Identifier: BSD-3-Clause
|
|
*
|
|
* https://opensource.org/license/bsd-3-clause
|
|
*
|
|
* Link-seam mock of the subset of libpq that dbd_pgsql.c uses. It lets the
|
|
* PostgreSQL driver be unit-tested with no server: every statement the driver
|
|
* issues is captured (command text, whether it was parameterized, and the bound
|
|
* parameter values) so the test can assert the driver keeps caller values out
|
|
* of the SQL text. SELECTs return an empty (0-row) result, which is all the
|
|
* tests need -- they assert the emitted command/params, not row contents.
|
|
*
|
|
* dbd_pgsql.c is compiled against the real <libpq-fe.h>, so these definitions
|
|
* must match the real prototypes; only the implementations are fake.
|
|
*/
|
|
|
|
#include <libpq-fe.h>
|
|
|
|
#include <stdio.h>
|
|
#include <string.h>
|
|
#include <strings.h>
|
|
|
|
#include "test_pgsql_stub.h"
|
|
|
|
/* opaque libpq types, completed here */
|
|
struct pg_conn {
|
|
int dummy;
|
|
};
|
|
struct pg_result {
|
|
ExecStatusType status;
|
|
};
|
|
|
|
static struct pg_conn g_conn;
|
|
static struct pg_result g_result;
|
|
|
|
#define PGSTUB_MAX_PARAMS 8
|
|
#define PGSTUB_STR 2048
|
|
|
|
static char g_command[PGSTUB_STR];
|
|
static int g_nparams;
|
|
static int g_used_params;
|
|
static char g_params[PGSTUB_MAX_PARAMS][PGSTUB_STR];
|
|
|
|
void pgstub_reset(void) {
|
|
g_command[0] = 0;
|
|
g_nparams = 0;
|
|
g_used_params = 0;
|
|
}
|
|
|
|
const char *pgstub_last_command(void) { return g_command; }
|
|
int pgstub_last_nparams(void) { return g_nparams; }
|
|
int pgstub_used_params(void) { return g_used_params; }
|
|
const char *pgstub_last_param(int i) {
|
|
if (i < 0 || i >= g_nparams || i >= PGSTUB_MAX_PARAMS) {
|
|
return NULL;
|
|
}
|
|
return g_params[i];
|
|
}
|
|
|
|
static struct pg_result *result_for(const char *command) {
|
|
const char *p = command;
|
|
while (*p == ' ' || *p == '\t' || *p == '\n' || *p == '\r') {
|
|
++p;
|
|
}
|
|
/* SELECTs return rows; everything else is a command. */
|
|
g_result.status = (strncasecmp(p, "select", 6) == 0) ? PGRES_TUPLES_OK : PGRES_COMMAND_OK;
|
|
return &g_result;
|
|
}
|
|
|
|
/////////////////////// connection ///////////////////////
|
|
|
|
ConnStatusType PQstatus(const PGconn *conn) {
|
|
(void)conn;
|
|
return CONNECTION_OK;
|
|
}
|
|
void PQfinish(PGconn *conn) { (void)conn; }
|
|
PGconn *PQconnectdb(const char *conninfo) {
|
|
(void)conninfo;
|
|
return &g_conn;
|
|
}
|
|
PQconninfoOption *PQconninfoParse(const char *conninfo, char **errmsg) {
|
|
(void)conninfo;
|
|
if (errmsg) {
|
|
*errmsg = NULL;
|
|
}
|
|
/* non-NULL so the driver proceeds; it only frees this, never reads it. */
|
|
static PQconninfoOption opt;
|
|
return &opt;
|
|
}
|
|
void PQconninfoFree(PQconninfoOption *connOptions) { (void)connOptions; }
|
|
char *PQerrorMessage(const PGconn *conn) {
|
|
(void)conn;
|
|
return (char *)"";
|
|
}
|
|
|
|
/////////////////////// statements ///////////////////////
|
|
|
|
PGresult *PQexec(PGconn *conn, const char *query) {
|
|
(void)conn;
|
|
g_used_params = 0;
|
|
g_nparams = 0;
|
|
snprintf(g_command, sizeof(g_command), "%s", query ? query : "");
|
|
return result_for(g_command);
|
|
}
|
|
|
|
PGresult *PQexecParams(PGconn *conn, const char *command, int nParams, const Oid *paramTypes,
|
|
const char *const *paramValues, const int *paramLengths, const int *paramFormats,
|
|
int resultFormat) {
|
|
(void)conn;
|
|
(void)paramTypes;
|
|
(void)paramLengths;
|
|
(void)paramFormats;
|
|
(void)resultFormat;
|
|
g_used_params = 1;
|
|
g_nparams = nParams;
|
|
snprintf(g_command, sizeof(g_command), "%s", command ? command : "");
|
|
for (int i = 0; i < nParams && i < PGSTUB_MAX_PARAMS; ++i) {
|
|
snprintf(g_params[i], sizeof(g_params[i]), "%s", (paramValues && paramValues[i]) ? paramValues[i] : "");
|
|
}
|
|
return result_for(g_command);
|
|
}
|
|
|
|
/////////////////////// results ///////////////////////
|
|
|
|
ExecStatusType PQresultStatus(const PGresult *res) { return res ? res->status : PGRES_FATAL_ERROR; }
|
|
int PQntuples(const PGresult *res) {
|
|
(void)res;
|
|
return 0;
|
|
}
|
|
char *PQgetvalue(const PGresult *res, int tup_num, int field_num) {
|
|
(void)res;
|
|
(void)tup_num;
|
|
(void)field_num;
|
|
return (char *)"";
|
|
}
|
|
int PQgetlength(const PGresult *res, int tup_num, int field_num) {
|
|
(void)res;
|
|
(void)tup_num;
|
|
(void)field_num;
|
|
return 0;
|
|
}
|
|
void PQclear(PGresult *res) { (void)res; }
|