corbel/FIX-NOTES-indoc-links.md

11 KiB

Patch notes — in-document links / false-positive heuristic fix

Problem

Operator-reported false positives while testing the heuristics against real documents containing in-document navigation:

  1. Markdown / EPUB / DOCX hyperlinks with fragment or relative-URL destinations (e.g. [Section 2](#section-2), [next chapter](./chapter2.html), [page](page.html#anchor)) were being flagged as Malicious(SuspiciousUri) — even though they point to another region of the same document and don't navigate anywhere external.

  2. mailto:user@example.com links (explicitly allowed in Config::allowed_uri_schemes) were also being flagged as Malicious(SuspiciousUri). This was the same bug surfacing in a different shape.

  3. PDF /GoTo actions (in-document page/destination jumps inside the same PDF) were conflated with /GoToR (remote navigation to another PDF file) at the parser level. Both were classified as Suspicious, so any PDF using bookmarks or table-of-contents links produced a noisy finding per clickable destination.

Root cause — classify_uri scheme parsing

The original classify_uri extracted the URI scheme with:

let scheme = uri
    .split("://")
    .next()
    .map(|s| s.to_ascii_lowercase())
    .unwrap_or_default();

split("://") only matches hierarchical schemes (https://, ftp://, file://). For anything else it returns the whole URI as the first segment, so the "scheme" became the entire URI string:

URI Parsed "scheme" Allowed? Result
https://example.com https yes Benign ✓
#section-2 #section-2 no Malicious ✗
./chapter2.html ./chapter2.html no Malicious ✗
mailto:user@example.com mailto:user@example.com no Malicious ✗
javascript:alert(1) javascript:alert(1) no Malicious ✓ (by luck — same result, wrong reason)

The intent of the original code was clearly "if there's no scheme, treat it as a relative URL and return Benign" — see the trailing fallback ThreatClassification::Benign at the end of classify_uri. But that fallback was unreachable in practice, because scheme was never actually empty when the URI contained any characters at all.

Root cause — PDF /GoTo vs /GoToR conflation

The PDF parser had:

"GoToR" | "GoTo" => {
    vectors.push(ExecutableVector {
        vector_type: VectorType::PdfGoToR,
        ...
    });
}

Both action types were emitted under the single VectorType::PdfGoToR variant, and the heuristics classified that variant as Suspicious. So /GoTo (in-document) and /GoToR (remote) were indistinguishable downstream.

Fix

1. Proper RFC 3986 scheme extraction

Added extract_uri_scheme(uri: &str) -> Option<&str> in src/scanner/heuristics.rs implementing the RFC 3986 §3.1 scheme grammar:

scheme = ALPHA *( ALPHA / DIGIT / "+" / "-" / "." ) ":"

The scheme is the longest prefix of uri that matches that grammar, ending at the first :. If no such prefix exists (i.e. there is no : in the URI, or the part before : doesn't match the scheme grammar), the URI has no scheme — it's an in-document link.

URI Extracted scheme In allowed list? Result
https://example.com Some("https") yes phishing check → Benign
#section-2 None n/a phishing check → Benign
./chapter2.html None n/a phishing check → Benign
page.html#anchor None n/a phishing check → Benign
mailto:user@example.com Some("mailto") yes phishing check → Benign
javascript:alert(1) Some("javascript") no Malicious ✓
data:text/html,<x> Some("data") no Malicious ✓
vbscript:msgbox Some("vbscript") no Malicious ✓

The user's intent — paraphrased — was:

An in-document link to another region of the same document shouldn't raise a flag. It should be used to check for other flags, but not raise an alert itself.

So classify_uri now routes scheme-less URIs through classify_phishing_signal(uri, config) (a small helper extracted from the original logic). If a strong phishing signal fires (BrandHomograph, IpHost, CredentialUrl) the URI is still classified as Malicious(SuspiciousUri). If a weak signal fires (Shortener, SuspiciousKeyword, PhishingTld) it's still Suspicious. Only when no phishing signal fires does the URI become Benign.

Examples of in-document links that STILL get flagged (correctly):

URI Phishing signal Result
#login-verify SuspiciousKeyword ("login", "verify") Suspicious
#micros0ft-attack-vector BrandHomograph ("micros0ft") Malicious
./page.html?account=verify SuspiciousKeyword Suspicious

3. PDF /GoTo (in-document) vs /GoToR (remote)

Added a new VectorType::PdfGoTo variant in src/core/types.rs (documented as "PDF /GoTo action — in-document navigation"). Updated the PDF parser to dispatch on the action type:

"GoTo"  => { /* emit VectorType::PdfGoTo   */ }
"GoToR" => { /* emit VectorType::PdfGoToR  */ }

Updated the heuristics classify_vector match:

// PDF /GoTo — in-document navigation. Benign.
VectorType::PdfGoTo => ThreatClassification::Benign,

// PDF /GoToR — remote navigation. Still Suspicious.
VectorType::PdfGoToR => ThreatClassification::Suspicious,

Because inspect_vector returns None for Benign findings, an in-document /GoTo produces no alert and no quarantine entry. The vector is still recorded in Document::executable_vectors so the operator can see in-document navigation activity if they want to.

Files changed

File Change
src/core/types.rs Added VectorType::PdfGoTo variant + "pdf-goto" display string. Updated doc comment on PdfGoToR to clarify it's for REMOTE navigation only.
src/parsers/pdf_parser.rs Split "GoToR" | "GoTo" match arm in inspect_action into two separate arms emitting PdfGoTo (in-document) and PdfGoToR (remote) respectively.
src/scanner/heuristics.rs Added VectorType::PdfGoTo => ThreatClassification::Benign case in classify_vector. Replaced broken split("://") scheme extraction with proper RFC 3986 extract_uri_scheme helper. Refactored phishing-signal escalation into classify_phishing_signal helper that's now called for BOTH scheme-bearing and scheme-less URIs (so in-document links still get phishing-checked).
src/scanner/heuristics.rs (tests) Added 10 new tests covering: RFC 3986 scheme extraction, fragment links, relative URLs, bare page links, mailto: links, in-document links with phishing signals, and the new PdfGoTo/PdfGoToR distinction.

Tests added

In src/scanner/heuristics.rs::tests:

Test name What it asserts
extract_uri_scheme_handles_rfc3986_cases The new scheme extractor handles hierarchical schemes, opaque schemes (mailto:, javascript:, data:, vbscript:), schemes with digits/+/-/., and correctly returns None for fragment links, relative URLs, and empty URIs.
fragment_link_is_benign #section (Markdown) → no finding emitted.
relative_url_is_benign ./page.html → no finding emitted.
bare_page_link_is_benign page.html → no finding emitted.
fragment_with_anchor_is_benign chapter1.html#section-2 → no finding emitted.
mailto_link_is_benign mailto:user@example.com → no finding emitted (was a false positive before the fix).
in_document_link_with_phishing_keyword_still_flagged #login-verify → Suspicious finding with suspicious-keyword note (proves in-document links still get phishing-checked).
in_document_link_with_brand_homograph_still_flagged #micros0ft-attack-vector → Malicious finding with brand-homograph note.
pdf_goto_in_document_navigation_is_benign VectorType::PdfGoTo → no finding emitted.
pdf_gotor_remote_navigation_is_suspicious VectorType::PdfGoToR → Suspicious (preserved behavior).

Test results

Before the fix:

  • cargo test --lib → 127 passed, 0 failed
  • cargo test --test pipeline_integration → 23 passed, 0 failed
  • cargo test --test zip_bomb_defense → 4 passed, 0 failed

After the fix:

  • cargo test --lib → 137 passed, 0 failed (+10 new tests)
  • cargo test --test pipeline_integration → 23 passed, 0 failed (unchanged)
  • cargo test --test zip_bomb_defense → 4 passed, 0 failed (unchanged)

All pre-existing tests continue to pass, including the suite that exercises real PDF / DOCX / EPUB / Markdown fixtures in tests/fixtures/.

Known limitations / non-goals

  1. Windows file paths like C:\path\to\file.txt will be parsed by extract_uri_scheme as having scheme "C" (because C is a valid RFC 3986 scheme character), and the heuristics will flag it as Malicious because "C" isn't in Config::allowed_uri_schemes. This is an acceptable false positive for an unusual input — Windows file paths should be encoded as file:///C:/path/to/file in URIs. The same applies to single-letter drive prefixes generally.

  2. The PDF parser doesn't yet capture /GoTo destination payloads. Both PdfGoTo and PdfGoToR vectors still have empty raw_payload. Capturing the /D (destination) entry for /GoTo and the /F (file reference) entry for /GoToR would let the scanner log where in-document navigation is actually pointing, but it's an enhancement — not required for the false-positive fix.

  3. Phishing-keyword matching against fragments is still substring based. #login-verify triggers SuspiciousKeyword because "login" and "verify" are both in SUSPICIOUS_URL_KEYWORDS and matching is lower.contains(kw). This is by design — the user explicitly said in-document links should still be checked for other flags. If substring matching proves too noisy on real documents, the keyword matcher in src/scanner/signatures.rs::match_suspicious_keyword can be tightened to word-boundary matching in a follow-up.