Files
coturn/tests/test_sqlite_support.c
bfacd81627 Merge commit from fork
* 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>
2026-05-24 09:56:03 -07:00

111 lines
3.3 KiB
C

/*
* SPDX-License-Identifier: BSD-3-Clause
*
* https://opensource.org/license/bsd-3-clause
*
* Minimal support/stub layer that lets tests/test_sqlite_dbd.c link
* src/apps/relay/dbdrivers/dbd_sqlite.c in isolation, without dragging in the
* whole turnserver. It provides just the relay/server symbols the SQLite
* driver references: the userdb path accessor, the per-thread connection key,
* the hex->binary key conversion, and trivial implementations of the realm /
* secrets-list / origin-map helpers the driver calls into.
*
* The same stubs are used for both the old (string-interpolated) and the new
* (parameterized) driver, so any behavior difference the tests observe comes
* from the driver under test, not from this layer.
*/
#include "mainrelay.h"
#include "dbdrivers/dbdriver.h"
#include <pthread.h>
#include <stdlib.h>
#include <string.h>
#include "test_sqlite_support.h"
/* Globals the driver expects to exist. mainrelay.c normally defines these; in
* the test we own them. */
turn_params_t turn_params;
pthread_key_t connection_key;
static persistent_users_db_t g_pud;
void test_sqlite_support_init(const char *dbpath) {
static int key_created = 0;
if (!key_created) {
(void)pthread_key_create(&connection_key, NULL);
key_created = 1;
}
memset(&g_pud, 0, sizeof(g_pud));
strncpy(g_pud.userdb, dbpath, sizeof(g_pud.userdb) - 1);
}
persistent_users_db_t *get_persistent_users_db(void) { return &g_pud; }
/* Verbatim from dbdriver.c (relay), which we do not link here. */
void convert_string_key_to_binary(const char *keysource, hmackey_t key, size_t sz) {
char is[3] = {0};
is[2] = 0;
for (size_t i = 0; i < sz; i++) {
is[0] = keysource[i * 2];
is[1] = keysource[i * 2 + 1];
unsigned int v = 0;
sscanf(is, "%02x", &v);
key[i] = (unsigned char)v;
}
}
/* --- secrets_list_t helpers: minimal append so the driver's list-producing
* functions can be observed by the test. --- */
void add_to_secrets_list(secrets_list_t *sl, const char *elem) {
if (!sl) {
return;
}
char **n = (char **)realloc(sl->secrets, (sl->sz + 1) * sizeof(char *));
if (!n) {
return;
}
sl->secrets = n;
sl->secrets[sl->sz++] = strdup(elem ? elem : "");
}
/* The driver forwards (kind table -> rows) into add_ip_list_range(); capture the
* ranges into a list the test can inspect. The ip_range_list_t argument is the
* test-owned (here ignored) sink. */
secrets_list_t g_test_ip_ranges;
int add_ip_list_range(const char *range, const char *realm, ip_range_list_t *list) {
(void)realm;
(void)list;
add_to_secrets_list(&g_test_ip_ranges, range);
return 0;
}
/* Only reached by sqlite_reread_realms(), which the tests do not exercise; these
* exist purely to satisfy the linker. */
static realm_params_t g_realm;
realm_params_t *get_realm(char *name) {
(void)name;
return &g_realm;
}
void lock_realms(void) {}
void unlock_realms(void) {}
void update_o_to_realm(ur_string_map *o_to_realm_new) { (void)o_to_realm_new; }
ur_string_map *ur_string_map_create(ur_string_map_func del_value_func) {
(void)del_value_func;
return NULL;
}
void ur_string_map_free(ur_string_map **map) {
if (map) {
*map = NULL;
}
}
bool ur_string_map_put(ur_string_map *map, const ur_string_map_key_type key, ur_string_map_value_type value) {
(void)map;
(void)key;
(void)value;
return true;
}