Skip to content

Commit

Permalink
feat(linter/eslint): implement no-new-func (#5360)
Browse files Browse the repository at this point in the history
Related to #479

---------

Co-authored-by: Don Isaac <[email protected]>
  • Loading branch information
shulaoda and DonIsaac authored Aug 31, 2024
1 parent 35f03db commit 1967c67
Show file tree
Hide file tree
Showing 3 changed files with 207 additions and 0 deletions.
2 changes: 2 additions & 0 deletions crates/oxc_linter/src/rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ mod eslint {
pub mod no_loss_of_precision;
pub mod no_multi_str;
pub mod no_new;
pub mod no_new_func;
pub mod no_new_native_nonconstructor;
pub mod no_new_wrappers;
pub mod no_nonoctal_decimal_escape;
Expand Down Expand Up @@ -524,6 +525,7 @@ oxc_macros::declare_all_lint_rules! {
eslint::no_iterator,
eslint::no_loss_of_precision,
eslint::no_new,
eslint::no_new_func,
eslint::no_new_wrappers,
eslint::no_nonoctal_decimal_escape,
eslint::no_obj_calls,
Expand Down
143 changes: 143 additions & 0 deletions crates/oxc_linter/src/rules/eslint/no_new_func.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
use oxc_ast::{ast::IdentifierReference, AstKind};
use oxc_diagnostics::OxcDiagnostic;
use oxc_macros::declare_oxc_lint;
use oxc_span::Span;

use crate::{context::LintContext, rule::Rule, AstNode};

fn no_new_func(span: Span) -> OxcDiagnostic {
OxcDiagnostic::warn("The Function constructor is eval.").with_label(span)
}

fn is_global_function_reference(ctx: &LintContext, id: &IdentifierReference) -> bool {
if let Some(reference_id) = id.reference_id() {
return id.name == "Function" && ctx.symbols().is_global_reference(reference_id);
}
false
}

#[derive(Debug, Default, Clone)]
pub struct NoNewFunc;

declare_oxc_lint!(
/// ### What it does
///
/// The rule disallow `new` operators with the `Function` object.
///
/// ### Why is this bad?
///
/// Using `new Function` or `Function` can lead to code that is difficult to understand and maintain. It can introduce security risks similar to those associated with `eval` because it generates a new function from a string of code, which can be a vector for injection attacks. Additionally, it impacts performance negatively as these functions are not optimized by the JavaScript engine.
///
/// ### Examples
///
/// Examples of **incorrect** code for this rule:
/// ```js
/// var x = new Function("a", "b", "return a + b");
/// var x = Function("a", "b", "return a + b");
/// var x = Function.call(null, "a", "b", "return a + b");
/// var x = Function.apply(null, ["a", "b", "return a + b"]);
/// var x = Function.bind(null, "a", "b", "return a + b")();
/// var f = Function.bind(null, "a", "b", "return a + b");
/// ```
///
/// Examples of **correct** code for this rule:
/// ```js
/// let x = function (a, b) {
/// return a + b;
/// };
/// ```
NoNewFunc,
style
);

impl Rule for NoNewFunc {
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
let id_and_span = match node.kind() {
AstKind::NewExpression(new_expr) => {
let Some(id) = new_expr.callee.get_identifier_reference() else {
return;
};

Some((id, new_expr.span))
}
AstKind::CallExpression(call_expr) => {
let Some(obj_id) = call_expr.callee.get_identifier_reference() else {
return;
};

Some((obj_id, call_expr.span))
}
AstKind::MemberExpression(mem_expr) => {
let parent: Option<&AstNode<'a>> =
ctx.nodes().iter_parents(node.id()).skip(1).find(|node| {
!matches!(
node.kind(),
AstKind::ChainExpression(_) | AstKind::ParenthesizedExpression(_)
)
});

let Some(AstKind::CallExpression(parent_call_expr)) = parent.map(AstNode::kind)
else {
return;
};

let Some(static_property_name) = &mem_expr.static_property_name() else {
return;
};

if !["apply", "bind", "call"].contains(static_property_name) {
return;
}

let Some(obj_id) = mem_expr.object().get_identifier_reference() else {
return;
};

Some((obj_id, parent_call_expr.span))
}
_ => None,
};

if let Some((id, span)) = id_and_span {
if is_global_function_reference(ctx, id) {
ctx.diagnostic(no_new_func(span));
}
}
}
}

#[test]
fn test() {
use crate::tester::Tester;

let pass = vec![
r#"var a = new _function("b", "c", "return b+c");"#,
r#"var a = _function("b", "c", "return b+c");"#,
"class Function {}; new Function()",
"const fn = () => { class Function {}; new Function() }",
"function Function() {}; Function()",
"var fn = function () { function Function() {}; Function() }",
"var x = function Function() { Function(); }",
"call(Function)",
"new Class(Function)",
"foo[Function]()",
"foo(Function.bind)",
"Function.toString()",
"Function[call]()",
];

let fail = vec![
r#"var a = new Function("b", "c", "return b+c");"#,
r#"var a = Function("b", "c", "return b+c");"#,
r#"var a = Function.call(null, "b", "c", "return b+c");"#,
r#"var a = Function.apply(null, ["b", "c", "return b+c"]);"#,
r#"var a = Function.bind(null, "b", "c", "return b+c")();"#,
r#"var a = Function.bind(null, "b", "c", "return b+c");"#,
r#"var a = Function["call"](null, "b", "c", "return b+c");"#,
r#"var a = (Function?.call)(null, "b", "c", "return b+c");"#,
"const fn = () => { class Function {} }; new Function('', '')",
"var fn = function () { function Function() {} }; Function('', '')",
];

Tester::new(NoNewFunc::NAME, pass, fail).test_and_snapshot();
}
62 changes: 62 additions & 0 deletions crates/oxc_linter/src/snapshots/no_new_func.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
---
source: crates/oxc_linter/src/tester.rs
---
eslint(no-new-func): The Function constructor is eval.
╭─[no_new_func.tsx:1:9]
1var a = new Function("b", "c", "return b+c");
· ────────────────────────────────────
╰────

eslint(no-new-func): The Function constructor is eval.
╭─[no_new_func.tsx:1:9]
1var a = Function("b", "c", "return b+c");
· ────────────────────────────────
╰────

eslint(no-new-func): The Function constructor is eval.
╭─[no_new_func.tsx:1:9]
1var a = Function.call(null, "b", "c", "return b+c");
· ───────────────────────────────────────────
╰────

eslint(no-new-func): The Function constructor is eval.
╭─[no_new_func.tsx:1:9]
1var a = Function.apply(null, ["b", "c", "return b+c"]);
· ──────────────────────────────────────────────
╰────

eslint(no-new-func): The Function constructor is eval.
╭─[no_new_func.tsx:1:9]
1var a = Function.bind(null, "b", "c", "return b+c")();
· ───────────────────────────────────────────
╰────

eslint(no-new-func): The Function constructor is eval.
╭─[no_new_func.tsx:1:9]
1var a = Function.bind(null, "b", "c", "return b+c");
· ───────────────────────────────────────────
╰────

eslint(no-new-func): The Function constructor is eval.
╭─[no_new_func.tsx:1:9]
1var a = Function["call"](null, "b", "c", "return b+c");
· ──────────────────────────────────────────────
╰────

eslint(no-new-func): The Function constructor is eval.
╭─[no_new_func.tsx:1:9]
1var a = (Function?.call)(null, "b", "c", "return b+c");
· ──────────────────────────────────────────────
╰────

eslint(no-new-func): The Function constructor is eval.
╭─[no_new_func.tsx:1:41]
1const fn = () => { class Function {} }; new Function('', '')
· ────────────────────
╰────

eslint(no-new-func): The Function constructor is eval.
╭─[no_new_func.tsx:1:50]
1var fn = function () { function Function() {} }; Function('', '')
· ────────────────
╰────

0 comments on commit 1967c67

Please sign in to comment.