Skip to content

Sanitize user input in AnalysisTriggerService and ThermalReferenceMetadataService log entries - #372

Merged
Christdej merged 1 commit into
equinor:mainfrom
Christdej:fix/codeql-log-forging-analysis-and-thermal
May 22, 2026
Merged

Sanitize user input in AnalysisTriggerService and ThermalReferenceMetadataService log entries#372
Christdej merged 1 commit into
equinor:mainfrom
Christdej:fix/codeql-log-forging-analysis-and-thermal

Conversation

@Christdej

@Christdej Christdej commented May 22, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes 13 open CodeQL cs/log-forging (CWE-117) code-scanning alerts by re-applying the existing Sanitize.SanitizeUserInput helper at the public service entry points, mirroring the established pattern from commit ac46c14 (fix(api): sanitize user input in PlantDataService log entries).

Why this is needed

The controllers and MQTT handlers already sanitize incoming user-controlled values, but CodeQL's cs/log-forging taint analysis does not follow the custom Sanitize.SanitizeUserInput helper across method/MQTT/DB boundaries. Re-applying the sanitizer at the service entry point keeps the data-flow local and recognized by the analysis, eliminating the alerts without changing observable behavior.

Changes

api/Services/AnalysisTriggerService.cs (10 alerts)

  • OnInspectionRecordCreated: sanitize inspectionRecord.InspectionId, createdEvent.AnalysisGroup.AnalysisGroupId, createdEvent.AnalysisGroup.AnalysisGroupAnalyses items, and createdEvent.RequiredAnalysis items at the top of the method (right after the null check on inspectionRecord).
  • RerunAnalysis: sanitize analysis.Name and each record.InspectionId after loading from the DB.
  • GetOrCreateAnalysisGroup: re-sanitize existing.GroupId when an existing group is loaded from the DB, since CodeQL still treats stored strings as tainted across the round-trip.

api/Services/ThermalReferenceMetadataService.cs (3 alerts)

  • CreateThermalReferenceMetadata and UpdateThermalReferenceMetadata: sanitize input.InstallationCode, input.TagId, input.InspectionDescription at the top of each method (so the sanitized values are also what get persisted to the DB — matching the PlantDataService.CreatePlantData pattern).
  • Added using api.Utilities;.

False positives?

None of the 13 alerts are false positives in CodeQL's threat model — all tainted values originate from [FromBody] HTTP requests or MQTT messages. The fixes resolve them properly rather than dismissing.

Verification

  • dotnet build api/api.csproj succeeds with 0 warnings / 0 errors.
  • No behavioral changes beyond stripping any \r/\n from the listed user-controlled strings (already done at the controller layer for most fields; this PR makes it visible to CodeQL and also defends in depth).

Code-scanning alerts addressed

ThermalReferenceMetadataService.cs:

AnalysisTriggerService.cs:

@Christdej
Christdej force-pushed the fix/codeql-log-forging-analysis-and-thermal branch 3 times, most recently from 5168595 to ab453b3 Compare May 22, 2026 10:11
@Christdej
Christdej force-pushed the fix/codeql-log-forging-analysis-and-thermal branch from ab453b3 to 3a3b6e3 Compare May 22, 2026 10:18
@Christdej Christdej changed the title fix(api): sanitize user input in AnalysisTriggerService and ThermalReferenceMetadataService log entries Sanitize user input in AnalysisTriggerService and ThermalReferenceMetadataService log entries May 22, 2026
@Christdej
Christdej force-pushed the fix/codeql-log-forging-analysis-and-thermal branch from 3a3b6e3 to 7104baf Compare May 22, 2026 10:23
@Christdej
Christdej merged commit 90bdcea into equinor:main May 22, 2026
8 checks passed
@Christdej
Christdej deleted the fix/codeql-log-forging-analysis-and-thermal branch May 22, 2026 10:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants