From d05f063d50e0db585da0fd2260c86dc4091e0f08 Mon Sep 17 00:00:00 2001 From: Brad Anderson Date: Tue, 28 Apr 2026 13:09:53 -0400 Subject: [PATCH] feat(rules/java): heuristic detection of custom authz service calls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Application-specific authorization layers (e.g. `PrivilegeService`, `AccessChecker`) expose boolean predicate methods like `isOrgAdmin`, `isCandidateManager`, `isAdminForAccount`, or `hasFullOrganizationAccess`. These don't match Spring/Shiro/Jakarta names so they were silently missed — even when they form the entire authorization layer of an app. Add a heuristic rule (`java-custom-authz-call`, confidence=medium, category=custom) that fires on `is*`/`has*`/`can*` method invocations whose names contain a role/access keyword as a complete camelCase sub-word: Admin, Manager, Member, Recruiter, Viewer, Editor, Owner, Access, Permission, Privilege, Role, Authority, Authorized Two predicates on the same `name: (identifier)` capture (using two capture aliases) implement an inclusive `match` plus an exclusive `not_match` so we don't double-report calls already handled by the framework rules (`hasRole`, `hasAnyRole`, `hasAuthority`, `hasAnyAuthority`, `isUserInRole`). The keyword regex requires a complete sub-word — a capital letter or end-of-name must follow the keyword — so `isIncludeAdmins` (where "Admin" is a substring of "Admins") is correctly rejected. Methods like `isArchived`, `isEmpty`, `hasLength`, `isValid` don't match either. Verified against the issue tarballs: vbench-server 3 → 115 findings (all 112 PrivilegeService call sites) endorsed_resume 11 → 16 findings (all 12 isOwner ownership checks) Hand-audited the new findings; no false positives surfaced. Adds 6 cargo-test cases and 9 toml `rule.tests` fixtures. Fixes #6. --- rules/java/custom-authz-call.toml | 135 ++++++++++++++++++++++++++++++ src/rules/embedded.rs | 4 + src/scanner/matcher.rs | 79 +++++++++++++++++ 3 files changed, 218 insertions(+) create mode 100644 rules/java/custom-authz-call.toml diff --git a/rules/java/custom-authz-call.toml b/rules/java/custom-authz-call.toml new file mode 100644 index 0000000..7d567d4 --- /dev/null +++ b/rules/java/custom-authz-call.toml @@ -0,0 +1,135 @@ +[rule] +id = "java-custom-authz-call" +languages = ["java"] +category = "custom" +confidence = "medium" +description = "Custom authorization-style method call (heuristic: name suggests role/membership/permission check)" +# Application-specific authorization layers (e.g. PrivilegeService, AccessChecker) +# expose boolean predicate methods that don't match Spring/Shiro/Jakarta names. +# This rule fires on `is*`/`has*`/`can*` method invocations whose names contain +# a role- or access-related keyword as a complete sub-word (e.g. `isOrgAdmin`, +# `isCandidateManager`, `isAdminForAccount`, `hasFullOrganizationAccess`). +# +# `method_name` is captured twice so we can apply both an inclusive `match` +# predicate and an exclusive `not_match` predicate that filters out names +# already covered by other rules (`hasRole`, `hasAuthority`, `isUserInRole`, +# their `hasAny*` variants). +# +# Confidence is `medium` because the pattern is heuristic — a manual review of +# each finding is expected. The category default Rego stub already prompts that. +query = """ +(method_invocation + name: (identifier) @method_name @method_name_excl +) @match +""" + +[rule.predicates.method_name] +# The keyword must be a complete camelCase word — followed by end-of-name +# or the start of another camelCase word ([A-Z]…). This avoids matching +# substrings of unrelated words (e.g. `isIncludeAdmins` where "Admin" is +# part of "Admins"). +match = "^(is|has|can)\\w*?(Admin|Manager|Member|Recruiter|Viewer|Editor|Owner|Access|Permission|Privilege|Role|Authority|Authorized)(?:[A-Z]\\w*)?$" + +[rule.predicates.method_name_excl] +not_match = "^(hasRole|hasAnyRole|hasAuthority|hasAnyAuthority|isUserInRole)$" + +[[rule.tests]] +input = """ +public class OrgController { + public void delete(Account account, Org org) { + if (!privService.isOrgAdmin(account.getId(), org.getId())) { + throw new ForbiddenException(); + } + } +} +""" +expect_match = true + +[[rule.tests]] +input = """ +public class CandidateController { + public void view(Account account, Candidate c) { + if (!privService.isCandidateViewer(account.getId(), c.getOrgId())) { + throw new ForbiddenException(); + } + } +} +""" +expect_match = true + +[[rule.tests]] +input = """ +public class AdminController { + public void edit(Account actor, Org org, Account subject) { + if (!privService.isAdminForAccount(actor, org, subject)) { + throw new ForbiddenException(); + } + } +} +""" +expect_match = true + +[[rule.tests]] +input = """ +public class ReportController { + public Report build(Account actor, Long orgId) { + if (!privService.hasFullOrganizationAccess(actor, orgId)) { + return Report.empty(); + } + return Report.full(); + } +} +""" +expect_match = true + +[[rule.tests]] +input = """ +public class Service { + public void doWork(Note note) { + if (note.isArchived()) { return; } + process(note); + } +} +""" +expect_match = false + +[[rule.tests]] +input = """ +public class Service { + public void doWork(String s) { + if (s.isEmpty()) { return; } + if (str.hasLength(s)) { process(s); } + } +} +""" +expect_match = false + +[[rule.tests]] +input = """ +public class SecurityConfig { + public void configure(HttpSecurity http) { + http.authorizeRequests().antMatchers("/admin/**").hasRole("ADMIN"); + } +} +""" +expect_match = false + +[[rule.tests]] +input = """ +public class Servlet { + public void doGet() { + if (request.isUserInRole("admin")) { allow(); } + } +} +""" +expect_match = false + +[[rule.tests]] +input = """ +public class Filter { + public boolean accept(SearchRequest req) { + return req.isIncludeAdmins() || req.isIncludeEmployees(); + } +} +""" +expect_match = false diff --git a/src/rules/embedded.rs b/src/rules/embedded.rs index dc149b3..1dceb0b 100644 --- a/src/rules/embedded.rs +++ b/src/rules/embedded.rs @@ -133,6 +133,10 @@ const EMBEDDED_RULES: &[(&str, &str)] = &[ "java-feature-gate-check", include_str!("../../rules/java/feature-gate-check.toml"), ), + ( + "java-custom-authz-call", + include_str!("../../rules/java/custom-authz-call.toml"), + ), ]; pub fn load_embedded_rules() -> Result> { diff --git a/src/scanner/matcher.rs b/src/scanner/matcher.rs index 94d8d83..b552a0c 100644 --- a/src/scanner/matcher.rs +++ b/src/scanner/matcher.rs @@ -637,6 +637,85 @@ public class MyService implements Serializable { ); } + #[test] + fn java_custom_authz_call_matches_role_suffix() { + let findings = parse_and_match_java( + r#"if (!privService.isOrgAdmin(account.getId(), org.getId())) { throw new ForbiddenException(); }"#, + include_str!("../../rules/java/custom-authz-call.toml"), + ); + assert!(!findings.is_empty(), "should match custom isOrgAdmin call"); + assert_eq!(findings[0].category, crate::types::AuthCategory::Custom); + } + + #[test] + fn java_custom_authz_call_matches_keyword_in_middle() { + let findings = parse_and_match_java( + r#"if (!privService.isAdminForAccount(actor, org, subject)) { throw new ForbiddenException(); }"#, + include_str!("../../rules/java/custom-authz-call.toml"), + ); + assert!( + !findings.is_empty(), + "should match isAdminForAccount (keyword in middle)" + ); + } + + #[test] + fn java_custom_authz_call_matches_has_access() { + let findings = parse_and_match_java( + r#"if (privService.hasFullOrganizationAccess(account, orgId)) { allow(); }"#, + include_str!("../../rules/java/custom-authz-call.toml"), + ); + assert!( + !findings.is_empty(), + "should match hasFullOrganizationAccess" + ); + } + + #[test] + fn java_custom_authz_call_no_substring_false_positive() { + // "Admin" appears as a substring of "Admins" — must NOT match. + let findings = parse_and_match_java( + r#"if (req.isIncludeAdmins()) { include(); }"#, + include_str!("../../rules/java/custom-authz-call.toml"), + ); + assert!( + findings.is_empty(), + "must not match isIncludeAdmins (Admin is a substring of Admins, not a complete sub-word)" + ); + } + + #[test] + fn java_custom_authz_call_no_state_check_false_positive() { + let findings = parse_and_match_java( + r#"if (note.isArchived()) { return; }"#, + include_str!("../../rules/java/custom-authz-call.toml"), + ); + assert!( + findings.is_empty(), + "must not match state-check methods like isArchived" + ); + } + + #[test] + fn java_custom_authz_call_excludes_known_framework_methods() { + // hasRole/hasAuthority/isUserInRole are handled by dedicated rules; + // this rule must NOT report them to avoid duplicate findings. + for snippet in [ + r#"http.authorizeRequests().antMatchers("/admin/**").hasRole("ADMIN");"#, + r#"http.authorizeRequests().antMatchers("/api/**").hasAuthority("SCOPE_read");"#, + r#"if (request.isUserInRole("admin")) { allow(); }"#, + ] { + let findings = parse_and_match_java( + snippet, + include_str!("../../rules/java/custom-authz-call.toml"), + ); + assert!( + findings.is_empty(), + "custom-authz-call must not duplicate framework rule for: {snippet}" + ); + } + } + #[test] fn java_feature_gate_matches() { let findings = parse_and_match_java(