Skip to content

[incubator-kie-issues#2361] fix log forging cwe#2347

Merged
yesamer merged 1 commit into
apache:mainfrom
AthiraHari77:incubator-kie-issues#2361
Jul 9, 2026
Merged

[incubator-kie-issues#2361] fix log forging cwe#2347
yesamer merged 1 commit into
apache:mainfrom
AthiraHari77:incubator-kie-issues#2361

Conversation

@AthiraHari77

Copy link
Copy Markdown
Contributor

Fixes : apache/incubator-kie-issues#2361

Sanitize newline characters in DataAuditQuery.toString() to prevent log forging (CWE-117) by replacing all line-break sequences (\R+) with _, including null-safety and Unicode line separators.

@josedee josedee requested a review from pefernan July 9, 2026 04:42

@josedee josedee left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment on lines -56 to +70
+ identifier.replace('\n', '_').replace('\r', '_')
+ sanitize(identifier)
+ ", graphQLDefinition="
+ graphQLDefinition.replace('\n', '_').replace('\r', '_')
+ sanitize(graphQLDefinition)
+ ", query="
+ query.replace('\n', '_').replace('\r', '_')
+ sanitize(query)
+ "]";
}

private static String sanitize(String value) {
if (value == null) {
return null;
}
return value.replaceAll("\\R+", "_");
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Athira. I haven't looked at the entire code, but this would also fix a possible NPE in the code.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses CWE-117 (log forging) risk by sanitizing line-break characters in DataAuditQuery.toString() output so log entries cannot be split/injected via CR/LF or other Unicode line separators.

Changes:

  • Centralized log-sanitization into a sanitize(String) helper.
  • Replaced manual \n/\r substitutions with \R+ regex replacement to cover all line-break sequences (including Unicode).
  • Added null-safety to avoid potential NullPointerException in toString().

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@yesamer yesamer merged commit fef169f into apache:main Jul 9, 2026
5 checks passed
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.

[CWE] SAST Low CWE-117 Log Forging

4 participants