Skip to content

Commit 634c6cb

Browse files
fix(cli): detect URL-encoded attacks and stop leaking Invalid Date in log-parser (#42)
Two correctness bugs in the log/attack parser (the actual product): 1. detectAttackPattern tested the raw nginx request path, which nginx logs URL-encoded. Plaintext signatures like '<script' or 'or 1=1' therefore never matched an encoded payload ('%3Cscript%3E', '%27%20OR%201=1') — a silent false negative in a security product. Now test the raw value and its URL-decoded forms (single + double encoding), guarding malformed '%' sequences. 2. parseNginxTimestamp/parseSyslogTimestamp wrapped new Date() in try/catch with a new Date() fallback, but new Date(<bad>) returns an Invalid Date instead of throwing, so the catch never fired and Invalid Date leaked into downstream time-window logic. Check getTime() explicitly. Also fix the syslog year assumption so a December log parsed in January is dated to the previous year rather than a full year in the future. Logic verified standalone (encoded SQLi/XSS/traversal detected, benign path still null, malformed input safe, timestamps never Invalid).
1 parent b740a19 commit 634c6cb

1 file changed

Lines changed: 39 additions & 14 deletions

File tree

apps/cli/src/core/log-parser.ts

Lines changed: 39 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -106,10 +106,31 @@ export function parseSyslog(line: string): SyslogEntry | null {
106106
}
107107

108108
export function detectAttackPattern(path: string): string | null {
109+
// nginx logs the request URI URL-encoded, so raw signatures like `<script`
110+
// or `or 1=1` never match an encoded payload (e.g. `%3Cscript%3E`,
111+
// `%27%20OR%201=1`). Test the raw value AND its URL-decoded forms so that
112+
// single- and double-encoded payloads are still caught. decodeURIComponent
113+
// throws on a malformed `%` sequence, so guard each decode.
114+
const candidates = new Set<string>([path]);
115+
let current = path;
116+
for (let i = 0; i < 2; i++) {
117+
let decoded: string | null = null;
118+
try {
119+
decoded = decodeURIComponent(current);
120+
} catch {
121+
decoded = null;
122+
}
123+
if (decoded === null || decoded === current) break;
124+
candidates.add(decoded);
125+
current = decoded;
126+
}
127+
109128
for (const [type, patterns] of Object.entries(ATTACK_PATTERNS)) {
110129
for (const pattern of patterns) {
111-
if (pattern.test(path)) {
112-
return type;
130+
for (const candidate of candidates) {
131+
if (pattern.test(candidate)) {
132+
return type;
133+
}
113134
}
114135
}
115136
}
@@ -131,20 +152,24 @@ export function autoDetectParser(line: string): ParsedLogLine | null {
131152

132153
function parseNginxTimestamp(s: string): Date {
133154
// "04/Apr/2026:12:00:00 +0000"
134-
try {
135-
const cleaned = s.replace(/(\d{2})\/(\w{3})\/(\d{4}):/, '$2 $1, $3 ');
136-
return new Date(cleaned);
137-
} catch {
138-
return new Date();
139-
}
155+
// new Date(<unparseable>) returns an Invalid Date instead of throwing, so the
156+
// old try/catch never triggered and an Invalid Date leaked into downstream
157+
// time-window logic. Check getTime() explicitly and fall back to now.
158+
const cleaned = s.replace(/(\d{2})\/(\w{3})\/(\d{4}):/, '$2 $1, $3 ');
159+
const d = new Date(cleaned);
160+
return Number.isNaN(d.getTime()) ? new Date() : d;
140161
}
141162

142163
function parseSyslogTimestamp(s: string): Date {
143-
// "Apr 4 12:00:00" — no year, assume current
144-
try {
145-
const withYear = `${s} ${new Date().getFullYear()}`;
146-
return new Date(withYear);
147-
} catch {
148-
return new Date();
164+
// "Apr 4 12:00:00" — no year. Assume the most recent year that is not in the
165+
// future, so a December log parsed in early January is dated to the previous
166+
// year rather than the current one.
167+
const now = new Date();
168+
let d = new Date(`${s} ${now.getFullYear()}`);
169+
if (Number.isNaN(d.getTime())) return now;
170+
if (d.getTime() > now.getTime()) {
171+
const prev = new Date(`${s} ${now.getFullYear() - 1}`);
172+
if (!Number.isNaN(prev.getTime())) d = prev;
149173
}
174+
return d;
150175
}

0 commit comments

Comments
 (0)