diff --git a/devdocs/modernization/NET10-PORT.md b/devdocs/modernization/NET10-PORT.md index 6d29832f..d3db144f 100644 --- a/devdocs/modernization/NET10-PORT.md +++ b/devdocs/modernization/NET10-PORT.md @@ -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 | diff --git a/server/AyaNova/DataList/DataListSqlFilterCriteriaBuilder.cs b/server/AyaNova/DataList/DataListSqlFilterCriteriaBuilder.cs index a08b557d..01b7bc65 100644 --- a/server/AyaNova/DataList/DataListSqlFilterCriteriaBuilder.cs +++ b/server/AyaNova/DataList/DataListSqlFilterCriteriaBuilder.cs @@ -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: