From 692fececc72544ddaef2d2800610b87b16a1ce54 Mon Sep 17 00:00:00 2001 From: John Cardinal Date: Sat, 24 Oct 2026 14:08:01 -0700 Subject: [PATCH] case 4716 - unit pick-list customer variant parsed as a number UnitPickList.GetVariantCriteria put the value of a "customerid:" variant from the request into the query unparsed, reachable by any login (security review F43, found during case 4712). The value is now parsed as a long; anything else means no restriction, as 0 already did. The client's format ("customerid: 5") still works. Checked: new test fails on the old code, passes now; pick-list tests 10/10. Co-Authored-By: Claude Opus 5.5 --- devdocs/modernization/NET10-PORT.md | 2 +- devdocs/modernization/SECURITY-REVIEW.md | 2 +- server/AyaNova/PickList/UnitPickList.cs | 6 ++++-- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/devdocs/modernization/NET10-PORT.md b/devdocs/modernization/NET10-PORT.md index 4e5940f8..9187c4c0 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. In 6 steps: 1 allowlist **done**; 2 work-order ownership **done**; 3 CSR and unit ownership **done**; 4 pick lists for contacts; 5 schedule staff rights; 6 notification subscriptions | | 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. **Done** (`@plq`/`@pltag` parameters; test `FetchUserPickListAutoCompleteApostrophe` fails on the old code, passes now; 9/9 pick-list tests) | +| 4716 | Pick-list search text as SQL parameters | F18, and F43 (unit pick-list variant parsed as a number, found during 4712, test `FetchUnitPickListCustomerVariant`). **Done** (`@plq`/`@pltag` parameters; test `FetchUserPickListAutoCompleteApostrophe` fails on the old code, passes now; 9/9 pick-list tests) | | 4717 | KPI interval allowlist | F26. **Done** (`KPIFetcher.DateTruncUnitOrNull` in the 7 KPIs reading `interval`; test `KpiInterval`) | | 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 d61db372..57816e81 100644 --- a/devdocs/modernization/SECURITY-REVIEW.md +++ b/devdocs/modernization/SECURITY-REVIEW.md @@ -189,7 +189,7 @@ take the next free number; a fixed or accepted finding keeps its ID and changes | ID | Severity | Status | Title | |---|---|---|---| -| F43 | **Critical** | confirmed (code) | Unit pick-list variant (`customerid:`) is put into SQL unparsed; any logged-in user can reach it | +| F43 | **Critical** | **fixed in case 4716** | Unit pick-list variant (`customerid:`) is put into SQL unparsed; any logged-in user can reach it | | F41 | **Critical** | **fixed in case 4710** (Sockeye; the business server gets it with 8.3.0) | F5 also in Sockeye (internet-facing admin login) and in the 8.2.x business AyaNova server | | F42 | High | confirmed (code) | The licence-signing RSA private keys are hardcoded in Sockeye's source and binaries | | F5 | **Critical** | **fixed in case 4710** | 2FA login step accepts an empty or missing temp token: the password is skipped for users with 2FA on | diff --git a/server/AyaNova/PickList/UnitPickList.cs b/server/AyaNova/PickList/UnitPickList.cs index 62b7698f..1447e977 100644 --- a/server/AyaNova/PickList/UnitPickList.cs +++ b/server/AyaNova/PickList/UnitPickList.cs @@ -139,8 +139,10 @@ namespace AyaNova.PickList switch (v[0]) { case "customerid": - if (string.IsNullOrWhiteSpace(v[1]) || v[1] == "0") return string.Empty; - return $"aunit.customerid = {v[1]}"; + //case 4716: the id goes into the SQL, so it must be a number (anything else: no restriction, as for 0) + if (!long.TryParse(v[1].Trim(), System.Globalization.NumberStyles.Integer, System.Globalization.CultureInfo.InvariantCulture, out long customerId) || customerId == 0) + return string.Empty; + return $"aunit.customerid = {customerId}"; } return string.Empty;