Basic auth checker - #83
Conversation
|
Thank you for your submission! Like many open source projects, we ask that you sign our CLA (Contributor License Agreement) before we can accept your contribution. Already signed the CLA? To re-check, try refreshing the page. |
…hen/.catch Extend FunctionAnalyzer::as_intrinsic so default/star @forge/api imports match both PropPath::Static(fetch) and PropPath::MemberCall(fetch). Method-call calles are normalized as MemberCall, so the previous Static-only arm missed some api.fetch(...) shapes. When lowering promise chains, the .then/.map/.catch shortcut inlined callbacks without lowering the receiver call, so api.fetch(...).then(...) never produced Intrinsic::Fetch in the IR. Recursively lower_call the inner CallExpr on the member object before inlining so fetch intrinsics are still emitted. Add fetch_http_basic_authorization_chained_api_fetch regression test. Fixes BasicAuth detection for apps that chain api.fetch (e.g. easybackup auth.js).
FSRT silently skipped entry points resolved through re-exporting index
files (e.g. export { handler } from './resolvers'). Three issues contributed:
1. ExportCollector::visit_named_export had all-empty match arms and
never called visit_children_with, so re-exports were never registered.
2. ImportCollector::visit_module_item only handled ModuleDecl::Import,
ignoring ModuleDecl::ExportNamed entirely.
3. run_checker only searched the entry point's module for
resolver.define() callbacks, missing callbacks in the re-exported
module.
Add a ReExportLinker visitor pass that runs after ExportCollector and
ImportCollector. For each NamedExport with a src field, it resolves the
source module, looks up the exported DefId, and registers it as an
export of the current module. Also extend run_checker to search the
def's owning module for resolver callbacks when it differs from the
entry module.
Adds a multi-file regression test and validates against
talive.assets.datatable, which now correctly reports the Basic auth
vulnerability in fetchAttributeList.
Reconstruct static URL fragments from template literals in value_from_rval instead of discarding them when expressions are Unknown. This preserves strings like 'api.atlassian.com/admin/v2/orgs//users' for downstream pattern matching. Generalize Bin::Add to preserve any known literal operand, not just Basic auth prefixes. Merge BasicAuthChecker and BearerAdminChecker into a single AuthHeaderChecker with an AuthHeaderVulnKind enum (BasicAuth, BearerAdmin). BasicAuth fires when the Authorization header starts with 'Basic ' and the fetch URL contains 'api.atlassian.com'. BearerAdmin fires when it starts with 'Bearer ' and the URL contains both 'api.atlassian.com' and 'admin'. Eliminates a redundant Interp instance and dataflow pass per function. Update SecretChecker to skip Bearer prefixes so that 'Bearer ' + unknown is not flagged as a hardcoded secret.
…hims
Previously, AuthHeaderChecker only inspected Intrinsic::Fetch (fetch/api.fetch) for Authorization headers. This change extends detection to all Forge platform API request shims:
- requestJira, requestConfluence, requestBitbucket (via api.asApp(), api. prefix, or direct named import)
- requestGraph (via api.asApp() or api. prefix)
- forgeFetch (bare call or api.forgeFetch)
Named imports from both @forge/api and @forge/bridge are now supported:
import { requestJira } from '@forge/api'
import { requestConfluence } from '@forge/bridge'
Key behavior changes:
- Platform API shims only check for BasicAuth (no BearerAdmin)
- No api.atlassian.com URL filter needed for platform shims (they are inherently Atlassian API calls)
- SecretChecker auth-prefix guard extended to avoid double-reporting on these shims
- Options argument index adapts for shims with >2 operands (e.g. requestGraph)
Added 7 regression tests covering api.asApp().requestJira, requestConfluence, requestBitbucket, forgeFetch, template literal Basic auth, @forge/bridge imports, and @forge/api named imports.
… issues By default, AuthHeaderChecker only scans code reachable from manifest entry points. This misses Authorization header misuse in class methods and helper functions that aren't on an entry-point call chain (e.g. requestJira with Basic auth inside an exported class method). Add a --scan-functions CLI flag and SCAN_FUNCTIONS env var that enables scanning all function and closure bodies for auth header vulnerabilities, regardless of entry-point reachability. Changes: - main.rs: Add scan_functions field to Args, gate full-function scan behind opts.scan_functions || SCAN_FUNCTIONS env var - interp.rs: Make EntryPoint, EntryKind, entry, try_check_function pub; add reset_dataflow_visited and is_dataflow_visited methods - definitions.rs: Add get_all_functions_and_closures() returning both Function and Closure DefIds (class methods are stored as Closure) - checkers.rs: Add vuln_count() and extend_vulns() helpers - test.rs: Add scan_directory_test_with_args helper; add tests for default-off and --scan-functions enabled behavior
…ssifier
Previously, AuthHeaderChecker only flagged Basic auth on bare fetch
calls when the URL literal contained 'api.atlassian.com'. This missed
the vast majority of real-world Atlassian endpoints that Forge apps
actually call: tenant URLs ending in .atlassian.net, customer wiki
endpoints, Bitbucket Cloud, Statuspage, Opsgenie, and the relative
paths produced when a template URL's ${baseUrl} substitution resolves
to the empty string (e.g. //rest/api/3/issue).
Add is_atlassian_url(&str) -> bool, a Rust port of the Python helper
in split_atlassian_urls.py, and use it in place of the old substring
check. The classifier supports:
- Host-suffix matching against an explicit Atlassian-owned suffix list
(atlassian.net, atlassian.com, jira-dev.com, atl-paas.net,
bitbucket.org, trello.com, statuspage.io, opsgenie.com, loom.com,
halp.com, mindville.com), dot-anchored to reject spoofy hosts like
atlassian.net.attacker.com.
- Templated/redacted subdomains, e.g. https://.atlassian.net/... and
https:///rest/api/3/myself, gated on host.is_empty() ||
host.starts_with('.') so the substring fallback can't be exploited
by a hostile authority.
- A regex of known Atlassian product REST path patterns (rest/api/{2,3,
latest}, rest/agile, rest/servicedeskapi, rest/insight, rest/forge,
rest/api/optics, rest/backup, wiki/{rest/api,api/v2}, ex/{jira,
confluence}, gateway/api/{graphql,jsm,public/teams,adf}, jsm/{assets,
csm,ops}, admin/v[12]/orgs, _edge/tenant_info), with an optional
leading slash so empty-baseUrl substitutions still match.
The BearerAdmin filter is intentionally left untouched and continues
to require api.atlassian.com && admin.
Also restrict the temporary FSRT_AUTH_URL_CSV logger so that only
Atlassian-bound call sites are written to the CSV: platform API
intrinsics (always Atlassian) and bare fetch calls whose resolved URL
satisfies is_atlassian_url. Non-Atlassian and unresolved-URL fetches
are now skipped, keeping the CSV focused on what's useful for
allowlist-building.
Tests live in crates/fsrt/src/test.rs (mod is_atlassian_url_tests),
covering full URLs, redacted/templated subdomains, relative paths
(including //rest/... empty-baseUrl form), spoofy/non-Atlassian hosts,
case-insensitivity, and the OpenAI/Azure cloudapp URLs that surfaced
during real-world CSV logging. All 59 tests pass.
removed to reduce added complexity in PR, TODO: Incorporate in later PR
…tion, remove PartialConst from value lattice Remove PartialConst variant from the Value enum and revert interp to return Unknown for partial concatenations. Instead, detect Basic/Bearer auth schemes directly in AuthHeaderChecker by walking the IR body's instructions to inspect BinOp(Add) and Template rvalues that produced the Authorization header value, following Read(Var) chains up to depth 4. Also scope the Authorization projection search to the specific headers VarId to prevent cross-contamination between call sites in the same function body. Extend extract_url_prefix_from_body to follow Read(Var) chains so template URLs like `https://api.atlassian.com/...` are correctly resolved. Remove is_basic_auth_concat_prefix and is_bearer_prefix from utils.rs. Fix fetch/forgeFetch/node-fetch URL check: all Intrinsic::Fetch calls now require URL validation against is_atlassian_url; only requestJira/Confluence/Bitbucket/Graph bypass the URL check as inherently Atlassian-bound.
880de57 to
eab0cfd
Compare
eab0cfd to
5e2c82d
Compare
| pub struct AuthHeaderVuln { | ||
| kind: AuthHeaderVulnKind, | ||
| stack: String, | ||
| entry_func: String, |
There was a problem hiding this comment.
Is this the entry function or the actual function where the API call happens?
There was a problem hiding this comment.
This should follow the same pattern as other scanners where the checker implements the Runner and AuthHeaderVuln implements IntoVuln (specifically for 2 vuln kinds AuthHeaderVulnKind::BasicAuth and AuthHeaderVulnKind::BearerAuth).
|
|
||
| match self.kind { | ||
| AuthHeaderVulnKind::BasicAuth => Vulnerability { | ||
| check_name: format!("Custom-Check-Basic-Authorization-{}", hasher.finish()), |
There was a problem hiding this comment.
Let's leave the custom check name hash off, so we can avoid filing another ticket for exemptions.
There was a problem hiding this comment.
Should I remove the check_name field entirely? removed the Custom-Check- prefix for now.
| ), | ||
| recommendation: "Prefer OAuth or API tokens scoped to least privilege. If Basic auth is required, load credentials from Forge secrets or environment variables and avoid logging or exposing the Authorization header.", | ||
| proof: format!( | ||
| "Basic Authorization header on fetch found via {}", |
There was a problem hiding this comment.
It might be better for debugging to have this be the API calls used
There was a problem hiding this comment.
I updated the proof to log the call type and endpoint checked.
| ( ^ | [^A-Za-z0-9_] ) | ||
| /? | ||
| ( | ||
| admin/v[12]/orgs (?: [^A-Za-z0-9_-] | $ ) |
There was a problem hiding this comment.
Are there any vulns that rely on this? This seems like it might be a common path.
There was a problem hiding this comment.
This is used to check the endpoint of a Bearer authorization call against patterns derived from the Admin Rest APIs. I believe around 8-10 forge apps use Admin Rest APIs in the findings.
eg. Basic Authorization header on fetch call to https:///rest/api/3/myself found via handler.validateCredentials
No description provided.