Skip to content

Sanitize user input at log call sites - #400

Merged
Christdej merged 1 commit into
equinor:mainfrom
Christdej:fix/codeql-log-forging-at-log-sites
Jun 12, 2026
Merged

Sanitize user input at log call sites#400
Christdej merged 1 commit into
equinor:mainfrom
Christdej:fix/codeql-log-forging-at-log-sites

Conversation

@Christdej

@Christdej Christdej commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Summary

Resolves the 13 open cs/log-forging (CWE-117) CodeQL alerts in AnalysisTriggerService.cs (10) and ThermalReferenceMetadataService.cs (3) by applying Sanitize.SanitizeUserInput directly at each logger.Log* call site.

Prior in-function upstream sanitization (e.g. input.X = Sanitize.SanitizeUserInput(input.X) at the top of service methods) is removed in favour of sanitizing only at the log call where the value is consumed. CodeQL's taint analysis loses the custom helper across .Select(...).ToList() projections, helper-method return values, DB round-trips, and private-helper parameters, so log-site sanitization is what reliably clears the alerts. Consolidating to log-site only also drops the duplication.

Controller-level entry-point sanitization added by #372/#398 is in different files and is unchanged.

Verification

  • dotnet build api/api.csproj: 0 warnings / 0 errors.
  • dotnet test: 88/88 pass.
  • Diff: +18 / −50 lines.

Code-scanning alerts addressed

ThermalReferenceMetadataService.cs:

AnalysisTriggerService.cs:

@Eddasol

Eddasol commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Should be covered by this PR: #398

Also, I would prefer if the commit message were a bit more to the point and followed the commit message best practice

@Christdej

Copy link
Copy Markdown
Contributor Author

Should be covered by this PR: #398

Also, I would prefer if the commit message were a bit more to the point and followed the commit message best practice

Yes, approved your PR. Lets merge that first and see what security scanning alterts remain.

@Christdej Christdej closed this Jun 12, 2026
@Christdej Christdej reopened this Jun 12, 2026
@Christdej
Christdej force-pushed the fix/codeql-log-forging-at-log-sites branch from 6f337bf to 631707d Compare June 12, 2026 07:06
@Christdej Christdej changed the title fix(api): sanitize user input at log call sites in AnalysisTriggerService and ThermalReferenceMetadataService Sanitize user input at log call sites Jun 12, 2026
@Christdej
Christdej force-pushed the fix/codeql-log-forging-at-log-sites branch from 631707d to 5c56cd6 Compare June 12, 2026 07:58
@Christdej
Christdej merged commit 945af88 into equinor:main Jun 12, 2026
8 checks passed
@Christdej
Christdej deleted the fix/codeql-log-forging-at-log-sites branch June 12, 2026 08:25
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.

3 participants