admin/raven
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

case 4676 - data-list numeric and roles filters: parse values, never append client text

Browse Source 
Roles and every numeric type (Decimal, Currency, Integer, InternalId, Enum,
MemorySize) now parse the filter value as an invariant decimal or long and
append the re-formatted number. An invalid value throws ArgumentException
before any SQL runs. Covers the types Sockeye's fix (3427cd8) missed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
John Cardinal
2026-10-22 14:36:28 -07:00
parent 627f1bc7ed
commit 1ca07f1321
2 changed files with 23 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
devdocs/modernization/NET10-PORT.md
View File

@@ -112,7 +112,7 @@ These are bugs, not port mechanics, so each is a separate commit that can be rev
Sockeye included its equivalents in the port branch. John decides per item whether they go
in this branch or a follow-up release. UPDATE: John has decided these all go in this branch and some more additional cases in the case database that are urgent, exact cases TBD.
- [ ] **Case 4676: data-list SQL injection — priority.** Confirmed: `DataListSqlFilterCriteriaBuilder` appends the raw filter value for Roles (`:343-354`) and for every numeric type (Enum, MemorySize, Decimal, Currency, InternalId, Integer, `:866-888`). Text (`:233`) and tag (`:1024`) values escape `'`; TimeSpan goes through `int.Parse` (`:1099+`). Any authenticated user who can open a data list can reach this. Port the Sockeye fix
- [x] **Case 4676: data-list SQL injection — priority.** Confirmed: `DataListSqlFilterCriteriaBuilder` appended the raw filter value for Roles and for every numeric type (Enum, MemorySize, Decimal, Currency, InternalId, Integer). Text and tag values escape `'`; TimeSpan goes through `int.Parse`. Any authenticated user who can open a data list could reach this. **Sockeye's fix (`3427cd8`) is incomplete:** it checks only Decimal/Currency and Integer, and leaves InternalId, Enum, MemorySize and Roles open (still open in Sockeye 8.1.0). AyaNova's fix parses every numeric type as an invariant `decimal` (Decimal, Currency) or `long` (the rest), and Roles as a `long`, then appends the re-formatted number. An invalid value throws `ArgumentException`, which the API returns as an error before any SQL runs
- [ ] **`DbUtil.AdminConnectionString`.** Confirmed: `DbUtil.cs:99` is `_dbConnectionString.Replace(_dbName, "postgres")`, used by the boot check (`:192`), create (`:286`) and drop/recreate (`:334`). Any customer whose role name, password or host contains the database name as text gets a mangled admin connection, and the boot check authenticates as the wrong user. Fix: `new NpgsqlConnectionStringBuilder(cs) { Database = "postgres" }.ConnectionString`
- [ ] **Backup `pg_dump` call.** Confirmed: `CoreJobBackup.cs:68` builds `postgresql://user:password@host:port/db` with the raw password, and `:94` adds `-Fc > file`; `RunProgram` runs it through `cmd.exe /C` (`RunProgram.cs:55`) or `/bin/sh -c` (`:82-89`). A password with spaces or `@ : / ? # %` breaks backups. Also, on Linux stderr is not redirected (`RunProgram.cs:86-92`), so a failed pg_dump returns empty output and is **reported as a successful backup** (`CoreJobBackup.cs:97`). Fix: password in `PGPASSWORD` in the process environment, connection parameters as separate arguments through `ProcessStartInfo.ArgumentList` (no shell), output with `-f`, stderr and exit code checked
- [ ] **Drop-and-recreate database.** Confirmed: `DbUtil.cs:342-345` drops, then creates; triggered by `AYANOVA_PERMANENTLY_ERASE_DATABASE` (`Startup.cs:567-570`). Without `CREATEDB` it drops the database and then fails to create it. Test the permission before dropping, or change the order
@@ -186,3 +186,4 @@ Found during the port, out of scope. One line each.
|---|---|---|
| 2026-10-22 | Survey of sections 1–3 against the code (read-only) | Findings recorded per item: n/a items ticked with reasons, evidence added, MiniProfiler found present |
| 2026-10-22 | Integration baseline, `eb607d34` (unmodified, .NET 8.0.24, fresh test database) | 103 tests: 96 passed, 2 failed (the two known server bugs), 5 skipped |
| 2026-10-22 | Case 4676 data-list SQL injection fix (.NET 8) | Integration suite same as baseline. API probe: valid Integer, Decimal and Roles filters return rows; `0 OR 1=1`, `0); select pg_sleep(3); --`, `1)=1 OR (1` and similar are rejected with `ArgumentException` before any SQL runs |

21
server/AyaNova/DataList/DataListSqlFilterCriteriaBuilder.cs
View File

@@ -338,6 +338,12 @@ namespace AyaNova.DataList
EQUALITY:
just a simple = or <> i.e. where roles=4096 or roles!=4096
*/
//case 4676 - roles are a bitmask, only a whole number is valid, never put client text into the query
if (!long.TryParse(sValue, NumberStyles.Integer, CultureInfo.InvariantCulture, out var roleValue))
throw new System.ArgumentException($"DataListSqlFilterCriteriaBuilder invalid ROLES filter value [{sValue}]");
sValue = roleValue.ToString(CultureInfo.InvariantCulture);
switch (sOperator)
{
case DataListFilterComparisonOperator.Equality:
@@ -861,6 +867,21 @@ namespace AyaNova.DataList
break;
}
//case 4676 - every numeric type must parse as a number, never put client text into the query
//Decimal and Currency as decimal, Integer, InternalId, Enum and MemorySize as whole numbers
if (DataType == UiFieldDataType.Decimal || DataType == UiFieldDataType.Currency)
{
if (!decimal.TryParse(sValue, NumberStyles.Number, CultureInfo.InvariantCulture, out var decimalValue))
throw new System.ArgumentException($"DataListSqlFilterCriteriaBuilder invalid NUMBER filter value [{sValue}]");
sValue = decimalValue.ToString(CultureInfo.InvariantCulture);
}
else
{
if (!long.TryParse(sValue, NumberStyles.Integer, CultureInfo.InvariantCulture, out var wholeValue))
throw new System.ArgumentException($"DataListSqlFilterCriteriaBuilder invalid NUMBER filter value [{sValue}]");
sValue = wholeValue.ToString(CultureInfo.InvariantCulture);
}
switch (sOperator)
{
case DataListFilterComparisonOperator.Equality: