Skip to content

Commit

Permalink
EAS-2582 : Map Scopes to OAuth
Browse files Browse the repository at this point in the history
  • Loading branch information
gersbach committed Jan 8, 2025
1 parent 7fa85d2 commit 1ae6c36
Show file tree
Hide file tree
Showing 5 changed files with 98 additions and 43 deletions.
8 changes: 8 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ Arguments:
-f, --function <FUNCTION> A specific function to scan. Must be an entrypoint specified in `manifest.yml`
-h, --help Print help information
-V, --version Print version information
--check-permissions Runs the permission checker
--graphql-schema-path <LOCATION> Uses the graphql schema in location; othwerwise selects ~/.config dir
```

## Installation
Expand Down Expand Up @@ -64,6 +66,12 @@ until then you can test `fsrt` by manually invoking:
fsrt ./test-apps/jira-damn-vulnerable-forge-app
```

Testing with a GraphQl Schema:

```sh
cargo test --features graphql_schema
```

## Contributions

Contributions to FSRT are welcome! Please see [CONTRIBUTING.md](CONTRIBUTING.md) for details.
Expand Down
33 changes: 17 additions & 16 deletions crates/forge_analyzer/src/checkers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use itertools::Itertools;
use smallvec::SmallVec;
use std::{
cmp::max,
collections::HashSet,
iter::{self, zip},
mem,
ops::ControlFlow,
Expand Down Expand Up @@ -1009,7 +1010,7 @@ impl PermissionDataflow {
}
}

impl WithCallStack for PermissionVuln {
impl<'a> WithCallStack for PermissionVuln<'a> {

Check failure on line 1013 in crates/forge_analyzer/src/checkers.rs

View workflow job for this annotation

GitHub Actions / Clippy

the following explicit lifetimes could be elided: 'a
fn add_call_stack(&mut self, _stack: Vec<DefId>) {}
}

Expand Down Expand Up @@ -1206,12 +1207,12 @@ impl<'cx> Dataflow<'cx> for PermissionDataflow {
}
}

pub struct PermissionChecker {
pub struct PermissionChecker<'a> {
pub visit: bool,
pub vulns: Vec<PermissionVuln>,
pub vulns: Vec<PermissionVuln<'a>>,
}

impl PermissionChecker {
impl<'a> PermissionChecker<'a> {
pub fn new() -> Self {
Self {
visit: false,
Expand All @@ -1221,22 +1222,22 @@ impl PermissionChecker {

pub fn into_vulns(
mut self,
permissions: Vec<String>,
) -> impl IntoIterator<Item = PermissionVuln> {
permissions: HashSet<&'a String>,
) -> impl IntoIterator<Item = PermissionVuln<'a>> {
if !permissions.is_empty() {
self.vulns.resize(1, PermissionVuln::new(permissions));
}
self.vulns
}
}

impl Default for PermissionChecker {
impl<'a> Default for PermissionChecker<'a> {

Check failure on line 1234 in crates/forge_analyzer/src/checkers.rs

View workflow job for this annotation

GitHub Actions / Clippy

the following explicit lifetimes could be elided: 'a
fn default() -> Self {
PermissionChecker::new()
}
}

impl fmt::Display for PermissionVuln {
impl<'a> fmt::Display for PermissionVuln<'a> {

Check failure on line 1240 in crates/forge_analyzer/src/checkers.rs

View workflow job for this annotation

GitHub Actions / Clippy

the following explicit lifetimes could be elided: 'a
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "Permission vulnerability")
}
Expand Down Expand Up @@ -1265,12 +1266,12 @@ impl JoinSemiLattice for PermissionTest {
}

#[derive(Debug, Clone)]
pub struct PermissionVuln {
unused_permissions: Vec<String>,
pub struct PermissionVuln<'a> {
unused_permissions: HashSet<&'a String>,
}

impl PermissionVuln {
pub fn new(unused_permissions: Vec<String>) -> PermissionVuln {
impl<'a> PermissionVuln<'a> {

Check failure on line 1273 in crates/forge_analyzer/src/checkers.rs

View workflow job for this annotation

GitHub Actions / Clippy

the following explicit lifetimes could be elided: 'a
pub fn new(unused_permissions: HashSet<&'_ String>) -> PermissionVuln<'_> {
PermissionVuln { unused_permissions }
}
}
Expand All @@ -1279,7 +1280,7 @@ pub struct DefinitionAnalysisRunner {
pub needs_call: Vec<(DefId, Vec<Operand>, Vec<Value>)>,
}

impl<'cx> Runner<'cx> for PermissionChecker {
impl<'cx, 'a> Runner<'cx> for PermissionChecker<'a> {

Check failure on line 1283 in crates/forge_analyzer/src/checkers.rs

View workflow job for this annotation

GitHub Actions / Clippy

the following explicit lifetimes could be elided: 'a
type State = PermissionTest;
type Dataflow = PermissionDataflow;

Expand All @@ -1295,11 +1296,11 @@ impl<'cx> Runner<'cx> for PermissionChecker {
}
}

impl Checker<'_> for PermissionChecker {
type Vuln = PermissionVuln;
impl<'a> Checker<'_> for PermissionChecker<'a> {
type Vuln = PermissionVuln<'a>;
}

impl IntoVuln for PermissionVuln {
impl<'a> IntoVuln for PermissionVuln<'a> {

Check failure on line 1303 in crates/forge_analyzer/src/checkers.rs

View workflow job for this annotation

GitHub Actions / Clippy

the following explicit lifetimes could be elided: 'a
fn into_vuln(self, reporter: &Reporter) -> Vulnerability {
Vulnerability {
check_name: "Least-Privilege".to_owned(),
Expand Down
3 changes: 3 additions & 0 deletions crates/fsrt/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ license.workspace = true
[lints]
workspace = true

[features]
graphql_schema = []

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
Expand Down
72 changes: 59 additions & 13 deletions crates/fsrt/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use std::{

use graphql_parser::{
query::{Mutation, Query, SelectionSet},
schema::ObjectType,
schema::{EnumType, EnumValue, ObjectType},
};

use graphql_parser::{
Expand Down Expand Up @@ -118,7 +118,7 @@ impl fmt::Display for Error {
}

struct PermissionsAndNextSelection<'a, 'b> {
permission_vec: Vec<String>,
permission_vec: Vec<&'a str>,
next_selection: NextSelection<'a, 'b>,
}

Expand All @@ -130,7 +130,7 @@ struct NextSelection<'a, 'b> {
fn parse_grapqhql_schema<'a: 'b, 'b>(
schema_doc: &'a [graphql_parser::schema::Definition<'a, &'a str>],
query_doc: &'b [graphql_parser::query::Definition<'b, &'b str>],
) -> Vec<String> {
) -> Vec<&'a str> {
let mut permission_list = vec![];

// dequeue of (parsed_query_selection: SelectionSet, schema_type_field: Field)
Expand Down Expand Up @@ -272,7 +272,7 @@ fn get_type_or_typex_with_name<'a, 'b>(
.flatten()
}

fn get_field_directives<'a>(field: &'a graphql_parser::schema::Field<'_, &'a str>) -> Vec<String> {
fn get_field_directives<'a>(field: &'a graphql_parser::schema::Field<'_, &'a str>) -> Vec<&'a str> {
let mut perm_vec = vec![];
field.directives.iter().for_each(|directive| {
if directive.name == "scopes" {
Expand All @@ -281,7 +281,7 @@ fn get_field_directives<'a>(field: &'a graphql_parser::schema::Field<'_, &'a str
if let query::Value::List(val) = &arg.1 {
val.iter().for_each(|val| {
if let query::Value::Enum(en) = val {
perm_vec.push(en.to_string());
perm_vec.push(*en);
}
});
}
Expand All @@ -295,7 +295,7 @@ fn get_field_directives<'a>(field: &'a graphql_parser::schema::Field<'_, &'a str
fn check_graphql_and_perms<'a>(
val: &'a Value,
path: &'a graphql_parser::schema::Document<'a, &'a str>,
) -> Vec<String> {
) -> Vec<&'a str> {
let mut operations = vec![];

match val {
Expand Down Expand Up @@ -488,7 +488,7 @@ pub(crate) fn scan_directory<'a>(
);
reporter.add_app(opts.appkey.clone().unwrap_or_default(), name.to_owned());

let mut perm_interp = Interp::<PermissionChecker>::new(
let mut perm_interp = Interp::<PermissionChecker<'_>>::new(
&proj.env,
false,
true,
Expand Down Expand Up @@ -618,6 +618,40 @@ pub(crate) fn scan_directory<'a>(
&mut path.to_owned()
};

let mut scope_path = path.clone();

scope_path.push("schema/shared/agg-shared-scopes.nadel");

let scope_map = fs::read_to_string(&scope_path).unwrap_or_default();

let ast = parse_schema::<&str>(&scope_map).unwrap_or_default();

let mut scope_name_to_oauth = HashMap::new();

ast.definitions.iter().for_each(|val| {
if let graphql_parser::schema::Definition::TypeDefinition(TypeDefinition::Enum(
EnumType { values, .. },
)) = val
{
values.iter().for_each(
|EnumValue {
directives, name, ..
}| {
if let Some(directive) = directives.first() {
if let graphql_parser::schema::Value::String(oauth_scope) = &directive
.arguments
.first()
.expect("Should only be one directive")
.1
{
scope_name_to_oauth.insert(*name, &**oauth_scope);
}
}
},
)
}
});

path.push("schema/*/*.nadel");

let joined_schema = glob(path.to_str().unwrap_or_default())
Expand All @@ -629,21 +663,21 @@ pub(crate) fn scan_directory<'a>(
let ast = parse_schema::<&str>(&joined_schema);

if let std::result::Result::Ok(doc) = ast {
let mut used_graphql_perms: Vec<String> = definition_analysis_interp
let mut used_graphql_perms: Vec<&str> = definition_analysis_interp
.value_manager
.varid_to_value_with_proj
.values()
.flat_map(|val| check_graphql_and_perms(val, &doc))
.collect();

let graphql_perms_varid: Vec<String> = definition_analysis_interp
let graphql_perms_varid: Vec<&str> = definition_analysis_interp
.value_manager
.varid_to_value
.values()
.flat_map(|val| check_graphql_and_perms(val, &doc))
.collect();

let graphql_perms_defid: Vec<String> = definition_analysis_interp
let graphql_perms_defid: Vec<&str> = definition_analysis_interp
.value_manager
.defid_to_value
.values()
Expand All @@ -653,14 +687,26 @@ pub(crate) fn scan_directory<'a>(
used_graphql_perms.extend_from_slice(&graphql_perms_defid);
used_graphql_perms.extend_from_slice(&graphql_perms_varid);

let final_perms: Vec<&String> = perm_interp
let oauth_scopes: HashSet<&str> = used_graphql_perms
.iter()
.copied()
.filter_map(|val| {
if !scope_name_to_oauth.contains_key(&val) {
warn!("Scope is not contained in the scope definitions")
}

scope_name_to_oauth.get(&val).copied()
})
.collect();

let final_perms: HashSet<&String> = perm_interp
.permissions
.iter()
.filter(|f| !used_graphql_perms.contains(&**f))
.filter(|f| !oauth_scopes.contains(f.as_str()))
.collect();

if run_permission_checker && !final_perms.is_empty() {
reporter.add_vulnerabilities([PermissionVuln::new(perm_interp.permissions)]);
reporter.add_vulnerabilities([PermissionVuln::new(final_perms)]);
}
}

Expand Down
Loading

0 comments on commit 1ae6c36

Please sign in to comment.