Write the quote as \x22 in twelve pcre patterns - #728
Merged
Conversation
Between_Quotes() in src/util.c does not stop at the second quote. Its
loop clears the copy flag on a quote and sets it again on the same
character, so what it returns is the whole option value minus its
quote characters. rules.c then reads the pattern from index 1 to the
first unescaped slash.
A quote inside a pattern is therefore deleted before PCRE ever sees
it, and any backslash in front of it attaches to the next character.
Twelve rules compile a pattern that is not the one on their line.
Measured against Sagan built from 3b9b0fa, one rule and one event at
a time. Each event satisfies the rest of the rule and carries text the
pattern as written accepts:
sid event ships patched
5009357 {\d}{\d}{\d}" -f silent alert
5007144 {1}{2}{3}" -f silent alert
5100137 logid="1234567890" silent alert
5017384 -Path "HKLM:...\Lsa" ... -Value "0" silent alert
5017389 the same, OutboundCreds silent alert
5017390 the same, Set-ItemProperty silent alert
5015511 "1a2b3c4d567 silent alert
5017383 reg add ...\Lsa /v ... /d "0" silent alert
5017387 the same, OutboundCreds silent alert
Nine rules do not fire on the text they describe. The [\"']? classes
lose their quote alternative and become [\']?, so a quoted registry
value is no longer matched; \" before a digit becomes \[, which asks
for a literal bracket.
The other three fire, and fire on text they do not describe:
sid event ships patched
5015124 xData":"payload.exe" alert silent
5015125 Data": "a: a: ... } (no closing ") alert silent
5014601 xprocdump.exe alert silent
sid 5014601 is the clearest: its option opens with / rather than a
quote, so Between_Quotes drops the slash and rules.c then drops the
first character of the pattern. The engine runs rocdump(64)*\.exe and
alerts on any path ending in rocdump.exe.
\x22 is the quote as PCRE reads it and is not a quote to the parser,
so it survives both steps. screenconnect.rules already writes its
quotes that way in the same patterns; this makes the remaining ones
consistent with it.
All twelve have their rev incremented.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Between_Quotes() in src/util.c does not stop at the second quote. Its loop clears the copy flag on a quote and sets it again on the same character, so what it returns is the whole option value minus its quote characters. rules.c then reads the pattern from index 1 to the first unescaped slash.
A quote inside a pattern is therefore deleted before PCRE ever sees it, and any backslash in front of it attaches to the next character. Twelve rules compile a pattern that is not the one on their line.
Measured against Sagan built from 3b9b0fa, one rule and one event at a time. Each event satisfies the rest of the rule and carries text the pattern as written accepts:
sid event ships patched
5009357 {\d}{\d}{\d}" -f silent alert
5007144 {1}{2}{3}" -f silent alert
5100137 logid="1234567890" silent alert
5017384 -Path "HKLM:...\Lsa" ... -Value "0" silent alert
5017389 the same, OutboundCreds silent alert
5017390 the same, Set-ItemProperty silent alert
5015511 "1a2b3c4d567 silent alert
5017383 reg add ...\Lsa /v ... /d "0" silent alert
5017387 the same, OutboundCreds silent alert
Nine rules do not fire on the text they describe. The ["']? classes lose their quote alternative and become [']?, so a quoted registry value is no longer matched; " before a digit becomes [, which asks for a literal bracket.
The other three fire, and fire on text they do not describe:
sid event ships patched
5015124 xData":"payload.exe" alert silent
5015125 Data": "a: a: ... } (no closing ") alert silent
5014601 xprocdump.exe alert silent
sid 5014601 is the clearest: its option opens with / rather than a quote, so Between_Quotes drops the slash and rules.c then drops the first character of the pattern. The engine runs rocdump(64)*.exe and alerts on any path ending in rocdump.exe.
\x22 is the quote as PCRE reads it and is not a quote to the parser, so it survives both steps. screenconnect.rules already writes its quotes that way in the same patterns; this makes the remaining ones consistent with it.
All twelve have their rev incremented.