Support BannedFunction messages for builtins and userland

slackhq · Jul 1, 2024 · 34062a4 · 34062a4
1 parent 77bca1c
commit 34062a4
Show file tree

Hide file tree

Showing 14 changed files with 117 additions and 18 deletions.
diff --git a/src/analyzer/config/json_config.rs b/src/analyzer/config/json_config.rs
@@ -11,6 +11,8 @@ pub struct JsonConfig {
     #[serde(default)]
     pub ignore_issue_files: FxHashMap<String, Vec<String>>,
     #[serde(default)]
+    pub banned_builtin_functions: FxHashMap<String, String>,
+    #[serde(default)]
     pub security_analysis: JsonSecurityConfig,
     #[serde(default)]
     pub allowed_issues: Vec<String>,

diff --git a/src/analyzer/config/mod.rs b/src/analyzer/config/mod.rs
@@ -5,7 +5,7 @@ use hakana_reflection_info::{
     issue::{Issue, IssueKind},
     taint::{SinkType, SourceType},
 };
-use hakana_str::Interner;
+use hakana_str::{Interner, StrId};
 use rustc_hash::{FxHashMap, FxHashSet};
 
 use crate::custom_hook::CustomHook;
@@ -26,6 +26,7 @@ pub struct Config {
     pub test_files: Vec<String>,
     pub ignore_issue_files: FxHashMap<IssueKind, Vec<String>>,
     pub ignore_all_issues_in_files: Vec<String>,
+    pub banned_builtin_functions: FxHashMap<StrId, StrId>,
     pub security_config: SecurityConfig,
     pub root_dir: String,
     pub hooks: Vec<Box<dyn CustomHook>>,
@@ -82,13 +83,15 @@ impl Config {
             ast_diff: false,
             in_migration: false,
             in_codegen: false,
+            banned_builtin_functions: FxHashMap::default(),
         }
     }
 
     pub fn update_from_file(
         &mut self,
         cwd: &String,
         config_path: &Path,
+        interner: &mut Interner,
     ) -> Result<(), Box<dyn Error>> {
         let json_config = json_config::read_from_file(config_path)?;
 
@@ -134,6 +137,12 @@ impl Config {
             )
         };
 
+        self.banned_builtin_functions = json_config
+            .banned_builtin_functions
+            .into_iter()
+            .map(|(k, v)| (interner.intern(k), interner.intern(v)))
+            .collect();
+
         self.security_config.ignore_files = json_config
             .security_analysis
             .ignore_files

diff --git a/src/analyzer/custom_hook.rs b/src/analyzer/custom_hook.rs
@@ -10,6 +10,7 @@ use oxidized::{
 };
 use rustc_hash::FxHashMap;
 
+use crate::config::Config;
 use crate::{
     config, function_analysis_data::FunctionAnalysisData, scope::BlockContext,
     statements_analyzer::StatementsAnalyzer,
@@ -154,7 +155,7 @@ pub trait InternalHook {
     }
 
     #[allow(unused_variables)]
-    fn after_populate(&self, codebase: &CodebaseInfo, interner: &Interner) {}
+    fn after_populate(&self, codebase: &CodebaseInfo, interner: &Interner, config: &Config) {}
 }
 
 pub trait CustomHook: InternalHook + Send + Sync + core::fmt::Debug {}
diff --git a/src/analyzer/expr/call/function_call_analyzer.rs b/src/analyzer/expr/call/function_call_analyzer.rs
@@ -14,8 +14,8 @@ use crate::expr::call_analyzer::{apply_effects, check_template_result};
 use crate::expr::{echo_analyzer, exit_analyzer, expression_identifier, isset_analyzer};
 use crate::function_analysis_data::FunctionAnalysisData;
 use crate::reconciler;
-use crate::scope_analyzer::ScopeAnalyzer;
 use crate::scope::BlockContext;
+use crate::scope_analyzer::ScopeAnalyzer;
 use crate::statements_analyzer::StatementsAnalyzer;
 use crate::stmt_analyzer::AnalysisError;
 use crate::{expression_analyzer, formula_generator};
@@ -202,6 +202,22 @@ pub(crate) fn analyze(
         &functionlike_id,
     );
 
+    if let Some(banned_message) = function_storage.banned_function_message {
+        analysis_data.maybe_add_issue(
+            Issue::new(
+                IssueKind::BannedFunction,
+                statements_analyzer
+                    .get_interner()
+                    .lookup(&banned_message)
+                    .to_string(),
+                statements_analyzer.get_hpos(pos),
+                &context.function_context.calling_functionlike_id,
+            ),
+            statements_analyzer.get_config(),
+            statements_analyzer.get_file_path_actual(),
+        );
+    }
+
     if !function_storage.is_production_code
         && function_storage.user_defined
         && context.function_context.is_production(codebase)

diff --git a/src/cli/lib.rs b/src/cli/lib.rs
@@ -683,8 +683,10 @@ fn do_fix(
 
     let config_path = config_path.unwrap();
 
+    let mut interner = Interner::default();
+
     if config_path.exists() {
-        config.update_from_file(&cwd, config_path).ok();
+        config.update_from_file(&cwd, config_path, &mut interner).ok();
     }
 
     config.allowed_issues = None;
@@ -698,6 +700,7 @@ fn do_fix(
         threads,
         Arc::new(logger),
         header,
+        interner,
         None,
         None,
         None,
@@ -731,8 +734,10 @@ fn do_remove_unused_fixmes(
 
     let config_path = config_path.unwrap();
 
+    let mut interner = Interner::default();
+
     if config_path.exists() {
-        config.update_from_file(cwd, config_path).ok();
+        config.update_from_file(cwd, config_path, &mut interner).ok();
     }
     config.allowed_issues = None;
 
@@ -750,6 +755,7 @@ fn do_remove_unused_fixmes(
         threads,
         Arc::new(logger),
         header,
+        interner,
         None,
         None,
         None,
@@ -805,8 +811,10 @@ fn do_add_fixmes(
 
     let config_path = config_path.unwrap();
 
+    let mut interner = Interner::default();
+
     if config_path.exists() {
-        config.update_from_file(cwd, config_path).ok();
+        config.update_from_file(cwd, config_path, &mut interner).ok();
     }
     config.allowed_issues = None;
 
@@ -821,6 +829,7 @@ fn do_add_fixmes(
         threads,
         Arc::new(logger),
         header,
+        interner,
         None,
         None,
         None,
@@ -868,8 +877,10 @@ fn do_migrate(
 
     let config_path = config_path.unwrap();
 
+    let mut interner = Interner::default();
+
     if config_path.exists() {
-        config.update_from_file(cwd, config_path).ok();
+        config.update_from_file(cwd, config_path, &mut interner).ok();
     }
     config.allowed_issues = None;
 
@@ -906,6 +917,7 @@ fn do_migrate(
         threads,
         Arc::new(logger),
         header,
+        interner,
         None,
         None,
         None,
@@ -954,8 +966,10 @@ fn do_migration_candidates(
 
     let config_path = config_path.unwrap();
 
+    let mut interner = Interner::default();
+
     if config_path.exists() {
-        config.update_from_file(cwd, config_path).ok();
+        config.update_from_file(cwd, config_path, &mut interner).ok();
     }
     config.allowed_issues = None;
 
@@ -972,6 +986,7 @@ fn do_migration_candidates(
         threads,
         Arc::new(logger),
         header,
+        interner,
         None,
         None,
         None,
@@ -1029,8 +1044,10 @@ fn do_codegen(
 
     let config_path = config_path.unwrap();
 
+    let mut interner = Interner::default();
+
     if config_path.exists() {
-        config.update_from_file(cwd, config_path).ok();
+        config.update_from_file(cwd, config_path, &mut interner).ok();
     }
     config.allowed_issues = None;
 
@@ -1045,6 +1062,7 @@ fn do_codegen(
         threads,
         Arc::new(logger),
         header,
+        interner,
         None,
         None,
         None,
@@ -1138,8 +1156,10 @@ fn do_find_paths(
 
     let config_path = config_path.unwrap();
 
+    let mut interner = Interner::default();
+
     if config_path.exists() {
-        config.update_from_file(cwd, config_path).ok();
+        config.update_from_file(cwd, config_path, &mut interner).ok();
     }
     config.allowed_issues = None;
 
@@ -1163,6 +1183,7 @@ fn do_find_paths(
         threads,
         Arc::new(logger),
         header,
+        interner,
         None,
         None,
         None,
@@ -1200,8 +1221,10 @@ fn do_security_check(
 
     let config_path = config_path.unwrap();
 
+    let mut interner = Interner::default();
+
     if config_path.exists() {
-        config.update_from_file(cwd, config_path).ok();
+        config.update_from_file(cwd, config_path, &mut interner).ok();
     }
     config.allowed_issues = None;
 
@@ -1227,6 +1250,7 @@ fn do_security_check(
         threads,
         Arc::new(logger),
         header,
+        interner,
         None,
         None,
         None,
@@ -1318,8 +1342,10 @@ fn do_analysis(
 
     let config_path = config_path.unwrap();
 
+    let mut interner = Interner::default();
+
     if config_path.exists() {
-        config.update_from_file(cwd, config_path).ok();
+        config.update_from_file(cwd, config_path, &mut interner).ok();
     }
 
     // do this after we've loaded from file, as they can be overridden
@@ -1344,6 +1370,7 @@ fn do_analysis(
         threads,
         Arc::new(logger),
         header,
+        interner,
         None,
         None,
         None,

diff --git a/src/cli/test_runners/test_runner.rs b/src/cli/test_runners/test_runner.rs
@@ -6,6 +6,7 @@ use hakana_reflection_info::code_location::FilePath;
 use hakana_reflection_info::data_flow::graph::GraphKind;
 use hakana_reflection_info::data_flow::graph::WholeProgramKind;
 use hakana_reflection_info::issue::IssueKind;
+use hakana_str::Interner;
 use hakana_workhorse::wasm::get_single_file_codebase;
 use hakana_workhorse::SuccessfulScanData;
 use rand::seq::SliceRandom;
@@ -232,6 +233,8 @@ impl TestRunner {
 
         let config = Arc::new(analysis_config);
 
+        let interner = Interner::default();
+
         let result = hakana_workhorse::scan_and_analyze(
             stub_dirs,
             None,
@@ -248,6 +251,7 @@ impl TestRunner {
             1,
             logger,
             build_checksum,
+            interner,
             previous_scan_data,
             previous_analysis_result,
             None,
@@ -457,6 +461,7 @@ impl TestRunner {
         let mut config = self.get_config_for_test(&workdir_base);
         config.ast_diff = true;
         config.find_unused_definitions = true;
+        let interner = Interner::default();
         let config = Arc::new(config);
         let mut stub_dirs = vec![cwd.clone() + "/tests/stubs"];
 
@@ -479,6 +484,7 @@ impl TestRunner {
                 1,
                 logger.clone(),
                 build_checksum,
+                interner.clone(),
                 previous_scan_data,
                 previous_analysis_result,
                 None,

diff --git a/src/code_info/functionlike_info.rs b/src/code_info/functionlike_info.rs
@@ -161,6 +161,8 @@ pub struct FunctionLikeInfo {
 
     pub is_production_code: bool,
 
+    pub banned_function_message: Option<StrId>,
+
     pub is_closure: bool,
 
     pub overriding: bool,
@@ -204,6 +206,7 @@ impl FunctionLikeInfo {
             has_throw: false,
             is_closure: false,
             overriding: false,
+            banned_function_message: None,
         }
     }
 

diff --git a/src/code_info_builder/functionlike_scanner.rs b/src/code_info_builder/functionlike_scanner.rs
@@ -366,6 +366,14 @@ pub(crate) fn get_functionlike(
             StrId::HAKANA_TEST_ONLY => {
                 functionlike_info.is_production_code = false;
             }
+            StrId::HAKANA_BANNED_FUNCTION => {
+                if let Some(attribute_param_expr) = user_attribute.params.first() {
+                    if let aast::Expr_::String(str) = &attribute_param_expr.2 {
+                        functionlike_info.banned_function_message =
+                            Some(interner.intern(str.to_string()));
+                    }
+                }
+            }
             StrId::HAKANA_SECURITY_ANALYSIS_IGNORE_PATH_IF_TRUE => {
                 functionlike_info.ignore_taints_if_true = true;
             }