Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions apps/cli/src/scan/__tests__/code-rules.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,39 @@ describe('guard windows', () => {
expect(ruleIds('a.java', bare)).toContain('java-xxe-parser-defaults');
});

it('flags an unhardened parse in a file that parses XML', () => {
const source = [
'import javax.xml.parsers.DocumentBuilder;',
'import javax.xml.parsers.DocumentBuilderFactory;',
'',
'public Document read(InputStream is) throws Exception {',
' return builder.parse(is);',
'}',
].join('\n');
expect(ruleIds('a.java', source)).toContain('java-xxe-parse-call');
});

// The receiver suffix alone matched any `.parse()` on anything named Builder,
// Parser or Reader. A hostname-mask parser is not an XML parser.
it('stays silent on a parser that has nothing to do with XML', () => {
const source = [
'package com.getcapacitor;',
'',
'public void setAllowedOrigins(String[] origins) {',
' this.mask = HostMask.Parser.parse(origins);',
'}',
].join('\n');
expect(ruleIds('a.java', source)).toHaveLength(0);
});

it('stays silent on a date parser and a JSON reader', () => {
const source = [
'LocalDate when = dateParser.parse(raw);',
'Config cfg = jsonReader.parse(body);',
].join('\n');
expect(ruleIds('a.java', source)).toHaveLength(0);
});

it('looks forward for an ObjectInputFilter installed after the stream', () => {
const filtered = [
'ObjectInputStream ois = new ObjectInputStream(new ByteArrayInputStream(blob));',
Expand Down
60 changes: 60 additions & 0 deletions apps/cli/src/scan/code-rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,18 @@ export interface CodeRule {
* harmless until something executes it.
*/
requires?: RegExp;
/**
* Evidence that must appear somewhere in the file, not merely in the guard
* window.
*
* For rules whose *applicability* is settled far from the match. Whether a
* `.parse()` call is XML parsing is decided by an import at the top of the
* file, which in a 1,600-line source is nowhere near the line that matched;
* widening `guardBack` far enough to reach it would drag in unrelated
* evidence for every other rule. Distinct from `requires`, which asks
* whether the surrounding lines complete a dangerous combination.
*/
fileRequires?: RegExp;
/**
* Evidence that the construct is already handled. `false` opts the rule out
* of the generic guard entirely — the CWE-532 rules are *about* reading
Expand Down Expand Up @@ -163,6 +175,17 @@ export const GENERIC_GUARD =
const XXE_GUARD =
/FEATURE_SECURE_PROCESSING|setExpandEntityReferences|disallow-doctype-decl|external-general-entities|external-parameter-entities|setXIncludeAware\s*\(\s*false/;

/**
* Evidence that a Java source parses XML at all.
*
* The package names are the reliable half: a file that reaches for
* `javax.xml.parsers` or `org.xml.sax` has declared its intent at the top,
* whatever the local variable ends up being called. The bare type names cover
* sources that import by wildcard or sit in the same package.
*/
const XML_PARSING_FILE =
/\b(?:javax\.xml|org\.xml\.sax|org\.w3c\.dom|org\.jdom2?|org\.dom4j|XmlPullParser|DocumentBuilderFactory|DocumentBuilder|SAXParserFactory|SAXParser|XMLInputFactory|XMLReaderFactory|XMLReader|SAXBuilder|SAXReader)\b/;

/** A sink that executes whatever string reaches it. */
const CODE_SINK =
/\bglobalThis\s*\[|\bconstructor\b|\beval\b|\bFunction\b|\brun\s*\(|\bvm\s*\.\s*run/;
Expand Down Expand Up @@ -507,6 +530,16 @@ export const CODE_RULES: readonly CodeRule[] = [
},

// ── XML external entities ────────────────────────────────────────────────
//
// Evidence that a file parses XML at all. The XXE rules match on receiver
// *names* — `builder.parse(is)` is the shape the vulnerability actually takes,
// and the declared type is rarely on that line — so without this the pattern
// reads any `.parse()` on anything suffixed Builder, Parser or Reader as XML.
// In practice that meant a hostname-mask parser (`HostMask.Parser.parse(...)`)
// was reported as CWE-611 at high severity.
//
// An import is the cheapest honest signal: a file that parses XML says so at
// the top, and one that never mentions XML is not parsing it.
{
id: 'java-xxe-parser-defaults',
title: 'XML parser left on its insecure defaults',
Expand All @@ -530,7 +563,10 @@ export const CODE_RULES: readonly CodeRule[] = [
severity: 'high',
languages: ['java'],
// Receiver-qualified so `LocalDate.parse(s)` and friends stay out of it.
// The suffix alone is not enough — plenty of parsers parse things that are
// not XML — so `fileRequires` decides whether the file is in scope at all.
pattern: /\b\w*(?:[Bb]uilder|[Pp]arser|[Rr]eader)\s*\.\s*parse\s*\(/,
fileRequires: XML_PARSING_FILE,
guard: XXE_GUARD,
guardBack: 6,
guardForward: 4,
Expand Down Expand Up @@ -802,6 +838,26 @@ function windowText(
return collected.join('\n');
}

/**
* Whole-file text, memoised on the `lines` array it came from.
*
* `fileRequires` asks a question no window can answer, but joining the file on
* every line of every rule would make scanning quadratic in file length. The
* caller already reuses one `lines` array for the whole file, so keying on its
* identity gives one join per file. A `WeakMap` keeps nothing alive after the
* file is done with.
*/
const FILE_TEXT = new WeakMap<readonly string[], string>();

function fileTextOf(lines: readonly string[]): string {
let text = FILE_TEXT.get(lines);
if (text === undefined) {
text = lines.join('\n');
FILE_TEXT.set(lines, text);
}
return text;
}

export interface RuleMatch {
rule: CodeRule;
confidence: Confidence;
Expand All @@ -821,6 +877,10 @@ export function evaluateRule(rule: CodeRule, ctx: MatchContext): RuleMatch | nul
if (isComment(line) || ctx.prose?.has(ctx.index)) return null;
if (!rule.pattern.test(line)) return null;

// Before any window work: a rule whose file-level precondition fails does not
// apply to this file at all.
if (rule.fileRequires && !rule.fileRequires.test(fileTextOf(ctx.lines))) return null;

const back = rule.guardBack ?? 8;
const forward = rule.guardForward ?? 0;
const context = windowText(ctx.lines, ctx.index, back, forward, ctx.prose);
Expand Down
Loading