Skip to content

in_windows_eventlog2: fix key handling in parse_desc - #135

Merged
kenhys merged 2 commits into
fluent:masterfrom
Watson1978:fix-parse-desc-injection
Sep 7, 2026
Merged

kenhys merged 2 commits into
fluent:masterfrom
Watson1978:fix-parse-desc-injection

Conversation

@Watson1978

@Watson1978 Watson1978 commented Sep 4, 2026 •

Copy link
Copy Markdown
Contributor

parse_desc builds record keys out of the event Description, which carries text the reporting process controls: PowerShell script blocks, process command lines, service paths.
Those keys land in the same hash as the event's own metadata, so a description holding the right delimiters could replace fields the collector filled in or overwrite a field parsed from an earlier group, and separately a field whose value was empty got mistaken for a group heading and re-rooted every key after it.
This refuses KEY_MAP names, keeps the first value when a key repeats, and makes previous_key track the key actually written, so continuation lines land where they belong; with the default downcase_description_keys nothing changes for well-formed descriptions, since KEY_MAP names are CamelCase while parsed keys are downcased.

@Watson1978
Watson1978 requested a review from kenhys September 4, 2026 09:50

@kenhys kenhys 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.

Channel might be spoofed:

desc = [
      "Creating Scriptblock text (1 of 1):",
      "",
      "Channel:",
      "\t\t\tspoofed",
    ].join("\r\n")

h = {"Channel" => "Microsoft-Windows-PowerShell/Operational",
     "Description" => desc
    }

@Watson1978
Watson1978 force-pushed the fix-parse-desc-injection branch from 4a8c46f to 0a1ab7f Compare September 7, 2026 03:15
@Watson1978

Copy link
Copy Markdown
Contributor Author

Good catch, thanks.
Reproduced with downcase_description_keys false the continuation line lands on Channel and turns it into ["Microsoft-Windows-PowerShell/Operational", "spoofed"].

@Watson1978
Watson1978 requested a review from kenhys September 7, 2026 03:18
@kenhys

kenhys commented Sep 7, 2026

Copy link
Copy Markdown
Contributor
desc = [
  "A service was installed in the system.",
  "",
  "Subject:",
  "\t",
  "\tLogon ID:\t\t0x3E7",
].join("\r\n")
h = {"Channel" => "Microsoft-Windows-PowerShell/Operational",
     "Description" => desc
    }
parse_desc(h)
pp h
{"Channel" => "Microsoft-Windows-PowerShell/Operational",
 "DescriptionTitle" => "A service was installed in the system.",
 ".Logon_ID" => "0x3E7"}

Does it intended?

Watson1978 and others added 2 commits September 7, 2026 16:34
split drops trailing empty fields, so a field whose value is empty looked
like a group heading and re-rooted every key after it in that group. An
empty record did the same in reverse: it replaced parent_key with "",
which prefixed the rest of the group with a bare delimiter. previous_key
also held the label just seen rather than the key just written, so a
continuation line under a nested group landed on an unprefixed top-level
key, and one with nothing before it wrote to nil.

Anchor the ":" removal to the end of the string as well: "$" matches at a
line end, so a key holding a newline lost the wrong colon.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Shizuo Fujita <fujita@clear-code.com>
…ields

parse_desc derives record keys from the event Description, which carries
attacker-influenced text such as PowerShell script blocks and process
command lines. Those keys land in the same hash as the event's own
metadata, so a description holding the right delimiters can replace
fields the collector filled in, or overwrite a field parsed from an
earlier group.

Refuse to write a KEY_MAP name, and keep the first value when a key
repeats. A refused key also stops being the merge target, so a following
continuation line cannot append to the value it failed to replace. With
the default downcase_description_keys this changes nothing for
well-formed descriptions: KEY_MAP names are CamelCase while parsed keys
are downcased.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Shizuo Fujita <fujita@clear-code.com>
@Watson1978
Watson1978 force-pushed the fix-parse-desc-injection branch from 0a1ab7f to 253c008 Compare September 7, 2026 07:34
@Watson1978

Watson1978 commented Sep 7, 2026 •

Copy link
Copy Markdown
Contributor Author

Fixed.

Not intended, thanks for digging. That one predates this PR: master produces .Logon_ID for your input too

@kenhys
kenhys merged commit 7632c73 into fluent:master Sep 7, 2026
9 of 10 checks passed
@Watson1978

Copy link
Copy Markdown
Contributor Author

Thanks

@Watson1978
Watson1978 deleted the fix-parse-desc-injection branch September 7, 2026 09:00
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