From 705a54f9d8fd736d695712d695434dc9dd496943 Mon Sep 17 00:00:00 2001 From: John Cardinal Date: Sat, 24 Oct 2026 12:10:00 -0700 Subject: [PATCH] case 4718 - report and translation import need create rights report/upload and translation/upload had no role check, so any login, customer contacts included, could import a report template (whose JavaScript runs in the server's browser at render time) or a translation (security review F19, F24). Both now require the same right as creating one, checked before the upload is read. The import screens are already limited to those roles. Checked locally: customer contact and Service 403, BizAdmin 202. Co-Authored-By: Claude Opus 5.5 --- devdocs/modernization/NET10-PORT.md | 2 +- devdocs/modernization/SECURITY-REVIEW.md | 4 ++-- server/AyaNova/Controllers/ReportController.cs | 3 +++ server/AyaNova/Controllers/TranslationController.cs | 3 +++ 4 files changed, 9 insertions(+), 3 deletions(-) diff --git a/devdocs/modernization/NET10-PORT.md b/devdocs/modernization/NET10-PORT.md index 458c8262..b13d46c3 100644 --- a/devdocs/modernization/NET10-PORT.md +++ b/devdocs/modernization/NET10-PORT.md @@ -184,7 +184,7 @@ John's "next release" cases for AyaNova (exported 2026-10-24). Each is its own s | 4715 | Download file names checked (one helper) | F21 | | 4716 | Pick-list search text as SQL parameters | F18 | | 4717 | KPI interval allowlist | F26 | -| 4718 | Report and translation import need create rights | F19, F24 | +| 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 | | 4720 | Restricted roles filtered in report/export data | F27 | | 4721 | Client escaping of record text | F33 | diff --git a/devdocs/modernization/SECURITY-REVIEW.md b/devdocs/modernization/SECURITY-REVIEW.md index 12669c57..b71d51c9 100644 --- a/devdocs/modernization/SECURITY-REVIEW.md +++ b/devdocs/modernization/SECURITY-REVIEW.md @@ -193,7 +193,7 @@ take the next free number; a fixed or accepted finding keeps its ID and changes | 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 | | F1 | Critical (installs that kept it) | **fixed in case 4711** | Placeholder JWT secret in the LAN/IIS and Linux server config templates | -| F19 | Critical | **confirmed** (import) | Any logged-in user, customer contacts included, can import report templates, whose JavaScript runs in the server's browser | +| 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 | candidate | 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 | | F26 | Critical | confirmed (code) | KPI `interval` criterion is put into SQL unchecked; reachable by staff users down to TechRestricted | @@ -208,7 +208,7 @@ take the next free number; a fixed or accepted finding keeps its ID and changes | F33 | High | confirmed (code) | Client renders some record text as HTML without escaping (search results, work-order address dialog, unit warranty info, dialogs) | | F3 | High | confirmed (code) | Any logged-in user, customer contacts included, can send arbitrary email through the company's SMTP server | | F23 | Medium | confirmed (code) | Logo upload accepts any content type and the anonymous logo route serves it inline from the app's origin (stored XSS) | -| F24 | Medium | confirmed (code) | Translation import has no role check: any login can add translations | +| F24 | Medium | **fixed in case 4718** | Translation import has no role check: any login can add translations | | F30 | Medium | confirmed (code) | SMTP: the mail server's TLS certificate is never checked | | F31 | Medium | confirmed (code) | The SMTP password is returned to read-only roles (OpsAdminRestricted, BizAdmin, BizAdminRestricted) | | F32 | Medium | confirmed (code) | A real-looking SMTP account and password are in the DEBUG defaults in `raven` and `sockeye` source | diff --git a/server/AyaNova/Controllers/ReportController.cs b/server/AyaNova/Controllers/ReportController.cs index 86dcdb03..849d9b2d 100644 --- a/server/AyaNova/Controllers/ReportController.cs +++ b/server/AyaNova/Controllers/ReportController.cs @@ -329,6 +329,9 @@ namespace AyaNova.Api.Controllers if (!serverState.IsOpen) return StatusCode(503, new ApiErrorResponse(serverState.ApiErrorCode, null, serverState.Reason)); + //case 4718: importing a template creates one, so it needs the same right as creating one (checked before the upload is read) + if (!Authorized.HasCreateRole(HttpContext.Items, AyaType.Report)) + return StatusCode(403, new ApiNotAuthorizedResponse()); // AyaTypeId attachToObject = null; ApiUploadProcessor.ApiUploadedFilesResult uploadFormData = null; diff --git a/server/AyaNova/Controllers/TranslationController.cs b/server/AyaNova/Controllers/TranslationController.cs index 85e1785a..2241e783 100644 --- a/server/AyaNova/Controllers/TranslationController.cs +++ b/server/AyaNova/Controllers/TranslationController.cs @@ -326,6 +326,9 @@ namespace AyaNova.Api.Controllers if (!serverState.IsOpen) return StatusCode(503, new ApiErrorResponse(serverState.ApiErrorCode, null, serverState.Reason)); + //case 4718: importing a translation creates one, so it needs the same right as creating one (checked before the upload is read) + if (!Authorized.HasCreateRole(HttpContext.Items, AyaType.Translation)) + return StatusCode(403, new ApiNotAuthorizedResponse()); // AyaTypeId attachToObject = null; ApiUploadProcessor.ApiUploadedFilesResult uploadFormData = null;