From b1944b7ca399d57a1b34c3a2ae5edcd4aa2e6ac9 Mon Sep 17 00:00:00 2001 From: John Cardinal Date: Sat, 24 Oct 2026 12:28:43 -0700 Subject: [PATCH] case 4716 - pick-list search text passed as SQL parameters PickListSqlBuilder put the search text and the tag search text into the SQL inside quotes with no escaping, reachable by any login (security review F18); an apostrophe in a search also made the query fail. The builder now writes @plq / @pltag and PickListFetcher adds them as Npgsql parameters (same %text% match; wildcards typed by the user still apply). Changelog: searching selection lists for text with an apostrophe now works. Co-Authored-By: Claude Opus 5.5 --- devdocs/modernization/NET10-PORT.md | 2 +- devdocs/modernization/SECURITY-REVIEW.md | 2 +- docs/8.0/ayanova/docs/changelog.md | 1 + server/AyaNova/PickList/PickListFetcher.cs | 5 +++++ server/AyaNova/PickList/PickListSqlBuilder.cs | 16 +++++++++------- 5 files changed, 17 insertions(+), 9 deletions(-) diff --git a/devdocs/modernization/NET10-PORT.md b/devdocs/modernization/NET10-PORT.md index 6030a825..7a6c760b 100644 --- a/devdocs/modernization/NET10-PORT.md +++ b/devdocs/modernization/NET10-PORT.md @@ -182,7 +182,7 @@ John's "next release" cases for AyaNova (exported 2026-10-24). Each is its own s | 4712 | Customer contact route allowlist + ownership checks | F13, F14, F15, F16, F17 | | 4713 | Contact users limited to customer roles | F28 | | 4715 | Download file names checked (one helper) | F21. **Done** (`FileUtil.FullPathInFolderOrNull` in temp/backup exists checks and both log routes; test `DownloadNames`) | -| 4716 | Pick-list search text as SQL parameters | F18 | +| 4716 | Pick-list search text as SQL parameters | F18. **Done** (`@plq`/`@pltag` parameters; test `FetchUserPickListAutoCompleteApostrophe` fails on the old code, passes now; 9/9 pick-list tests) | | 4717 | KPI interval allowlist | F26 | | 4718 | Report and translation import need create rights | F19, F24. **Done** (contact and Service 403, BizAdmin 202; test `ImportRights`) | | 4719 | Report page network restricted; notification reports not run as SuperUser | F20 | diff --git a/devdocs/modernization/SECURITY-REVIEW.md b/devdocs/modernization/SECURITY-REVIEW.md index 563ff699..2bb9d12d 100644 --- a/devdocs/modernization/SECURITY-REVIEW.md +++ b/devdocs/modernization/SECURITY-REVIEW.md @@ -195,7 +195,7 @@ take the next free number; a fixed or accepted finding keeps its ID and changes | F1 | Critical (installs that kept it) | **fixed in case 4711** | Placeholder JWT secret in the LAN/IIS and Linux server config templates | | F19 | Critical | **fixed in case 4718** | Any logged-in user, customer contacts included, can import report templates, whose JavaScript runs in the server's browser | | F21 | Critical | **fixed in case 4715** | Download file names from the URL are not checked: `report/download` and `export/download` may serve files outside the temp folder on Windows | -| F18 | Critical | confirmed (code) | Pick-list search text is put into SQL without escaping; any logged-in user can reach it | +| F18 | Critical | **fixed in case 4716** | Pick-list search text is put into SQL without escaping; any logged-in user can reach it | | F26 | Critical | confirmed (code) | KPI `interval` criterion is put into SQL unchecked; reachable by staff users down to TechRestricted | | F13 | High | **confirmed** | Customer contacts can open any customer's work order (customer view, customer report, attachments) by id | | F14 | High | **confirmed** | Customer contacts can read, and by code change, delete and create, any customer's service requests | diff --git a/docs/8.0/ayanova/docs/changelog.md b/docs/8.0/ayanova/docs/changelog.md index 97f6361b..41d418be 100644 --- a/docs/8.0/ayanova/docs/changelog.md +++ b/docs/8.0/ayanova/docs/changelog.md @@ -34,6 +34,7 @@ IN PROGRESS **Fixed** +- Server: searching selection lists for text that contains an apostrophe (for example a name like O'Brien) now works. - Server: hardened data table filter handling of number and role filter values. - Server: data table date filters for "greater than or equal", "less than" and "not equal" could include or exclude records on the boundary of the selected date. - Server: the database connection used for administrative operations is now built correctly when the database name also appears elsewhere in the connection setting. diff --git a/server/AyaNova/PickList/PickListFetcher.cs b/server/AyaNova/PickList/PickListFetcher.cs index 4225b75b..e01e52ea 100644 --- a/server/AyaNova/PickList/PickListFetcher.cs +++ b/server/AyaNova/PickList/PickListFetcher.cs @@ -56,6 +56,11 @@ namespace AyaNova.PickList //GET DATA RETURN ROWS command.CommandText = q; + //case 4716: search texts go in as parameters (same %text% match as before, wildcards typed by the user still apply) + if (q.Contains("@plq")) + command.Parameters.Add(new Npgsql.NpgsqlParameter("plq", $"%{autoCompleteQuery}%")); + if (q.Contains("@pltag")) + command.Parameters.Add(new Npgsql.NpgsqlParameter("pltag", $"%{tagSpecificQuery}%")); try { using (var dr = await command.ExecuteReaderAsync()) diff --git a/server/AyaNova/PickList/PickListSqlBuilder.cs b/server/AyaNova/PickList/PickListSqlBuilder.cs index a0f5053f..a0415c89 100644 --- a/server/AyaNova/PickList/PickListSqlBuilder.cs +++ b/server/AyaNova/PickList/PickListSqlBuilder.cs @@ -104,13 +104,15 @@ namespace AyaNova.PickList } } + //case 4716: the search texts are SQL parameters (@plq, @pltag), added by PickListFetcher, never part of the SQL text + //PROCESS TAG SPECIFIC QUERY // if (HasTagSpecificQuery) { //get the tag column AyaPickListFieldDefinition tagColumn = pickList.ColumnDefinitions.FirstOrDefault(z => z.ColumnDataType == UiFieldDataType.Tags); - TagSpecificWhereFragment = $"(array_to_string({tagColumn.GetSqlValueColumnName()},',') like '%{tagSpecificQuery}%')"; + TagSpecificWhereFragment = $"(array_to_string({tagColumn.GetSqlValueColumnName()},',') like @pltag)"; } @@ -150,9 +152,9 @@ namespace AyaNova.PickList if (HasAutoCompleteQuery && !HasTagSpecificQuery) { if (ServerGlobalBizSettings.Cache.FilterCaseSensitive) - sWhere = $"(array_to_string({valueColumnName},',') like '%{autoCompleteQuery}%')"; + sWhere = $"(array_to_string({valueColumnName},',') like @plq)"; else - sWhere = $"(lower(array_to_string({valueColumnName},',')) like lower('%{autoCompleteQuery}%'))"; + sWhere = $"(lower(array_to_string({valueColumnName},',')) like lower(@plq))"; } @@ -165,9 +167,9 @@ namespace AyaNova.PickList lOrderBy.Add(valueColumnName); if (HasAutoCompleteQuery) if (ServerGlobalBizSettings.Cache.FilterCaseSensitive) - sWhere = $"({valueColumnName} like '%{autoCompleteQuery}%')"; + sWhere = $"({valueColumnName} like @plq)"; else - sWhere = $"(lower({valueColumnName}) like lower('%{autoCompleteQuery}%'))"; + sWhere = $"(lower({valueColumnName}) like lower(@plq))"; } else @@ -187,9 +189,9 @@ namespace AyaNova.PickList //(cast (aworkorder.serial as text) like '%some%') if (HasAutoCompleteQuery) if (ServerGlobalBizSettings.Cache.FilterCaseSensitive) - sWhere = $"(cast ({valueColumnName} as text) like '%{autoCompleteQuery}%')"; + sWhere = $"(cast ({valueColumnName} as text) like @plq)"; else - sWhere = $"(lower(cast ({valueColumnName} as text)) like lower('%{autoCompleteQuery}%'))"; + sWhere = $"(lower(cast ({valueColumnName} as text)) like lower(@plq))"; }