Skip to content
This repository was archived by the owner on Aug 31, 2023. It is now read-only.

Commit e0325d3

Browse files
unvalleyematipico
unvalley
and
ematipico
authored
feat(rome_js_analyze): noPrototypeBuiltins (#4101)
Co-authored-by: Emanuele Stoppa <[email protected]> Closes #3979
1 parent e49cbe4 commit e0325d3

File tree

13 files changed

+531
-7
lines changed

13 files changed

+531
-7
lines changed

‎crates/rome_diagnostics_categories/src/categories.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ define_dategories! {
9595
"lint/nursery/noDuplicateJsxProps": "https://docs.rome.tools/lint/rules/noDuplicateJsxProps",
9696
"lint/nursery/useYield": "https://docs.rome.tools/lint/rules/useYield",
9797
"lint/nursery/noGlobalObjectCalls": "https://docs.rome.tools/lint/rules/noGlobalObjectCalls",
98+
"lint/nursery/noPrototypeBuiltins": "https://docs.rome.tools/lint/rules/noPrototypeBuiltins",
9899
// Insert new nursery rule here
99100

100101
// performance

‎crates/rome_js_analyze/src/analyzers/nursery.rs‎

Lines changed: 2 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎crates/rome_js_analyze/src/analyzers/nursery/no_prototype_builtins.rs‎

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
use rome_analyze::{context::RuleContext, declare_rule, Ast, Rule, RuleDiagnostic};
2+
use rome_console::markup;
3+
use rome_js_syntax::{AnyJsExpression, JsCallExpression, TextRange};
4+
use rome_rowan::AstNodeList;
5+
6+
declare_rule! {
7+
/// Disallow direct use of `Object.prototype` builtins.
8+
///
9+
/// ECMAScript 5.1 added `Object.create` which allows the creation of an object with a custom prototype.
10+
/// This pattern is often used for objects used as Maps. However, this pattern can lead to errors
11+
/// if something else relies on prototype properties/methods.
12+
/// Moreover, the methods could be shadowed, this can lead to random bugs and denial of service
13+
/// vulnerabilities. For example, calling `hasOwnProperty` directly on parsed JSON like `{"hasOwnProperty": 1}` could lead to vulnerabilities.
14+
/// To avoid subtle bugs like this, you should call these methods from `Object.prototype`.
15+
/// For example, `foo.isPrototypeof(bar)` should be replaced with `Object.prototype.isPrototypeof.call(foo, "bar")`
16+
/// As for the `hasOwn` method, `foo.hasOwn("bar")` should be replaced with `Object.hasOwn(foo, "bar")`.
17+
///
18+
/// ## Examples
19+
///
20+
/// ### Invalid
21+
///
22+
/// ```js,expect_diagnostic
23+
/// var invalid = foo.hasOwnProperty("bar");
24+
/// ```
25+
///
26+
/// ```js,expect_diagnostic
27+
/// var invalid = foo.isPrototypeOf(bar);
28+
/// ```
29+
///
30+
/// ```js,expect_diagnostic
31+
/// var invalid = foo.propertyIsEnumerable("bar");
32+
/// ```
33+
///
34+
/// ## Valid
35+
///
36+
/// ```js
37+
/// var valid = Object.hasOwn(foo, "bar");
38+
/// var valid = Object.prototype.isPrototypeOf.call(foo, bar);
39+
/// var valid = {}.propertyIsEnumerable.call(foo, "bar");
40+
/// ```
41+
///
42+
pub(crate) NoPrototypeBuiltins {
43+
version: "next",
44+
name: "noPrototypeBuiltins",
45+
recommended: false,
46+
}
47+
}
48+
49+
pub(crate) struct RuleState {
50+
prototype_builtins_method_name: String,
51+
text_range: TextRange,
52+
}
53+
54+
impl Rule for NoPrototypeBuiltins {
55+
type Query = Ast<JsCallExpression>;
56+
type State = RuleState;
57+
type Signals = Option<Self::State>;
58+
type Options = ();
59+
60+
fn run(ctx: &RuleContext<Self>) -> Self::Signals {
61+
let call_expr = ctx.query();
62+
let callee = call_expr.callee().ok()?.omit_parentheses();
63+
64+
match callee {
65+
AnyJsExpression::JsComputedMemberExpression(expr) => {
66+
let expr = expr.member().ok()?;
67+
match expr {
68+
AnyJsExpression::AnyJsLiteralExpression(expr) => {
69+
let literal_expr = expr.as_js_string_literal_expression()?;
70+
let token_text = literal_expr.inner_string_text().ok()?;
71+
let token = literal_expr.value_token().ok()?;
72+
73+
is_prototype_builtins(token_text.text()).then_some(RuleState {
74+
prototype_builtins_method_name: token_text.to_string(),
75+
text_range: token.text_range(),
76+
})
77+
}
78+
AnyJsExpression::JsTemplateExpression(expr) => {
79+
let template_element = expr.as_fields().elements.first()?;
80+
let template_chunk_token = template_element
81+
.as_js_template_chunk_element()?
82+
.template_chunk_token()
83+
.ok()?;
84+
let token_text = template_chunk_token.text();
85+
is_prototype_builtins(token_text).then_some(RuleState {
86+
prototype_builtins_method_name: token_text.to_string(),
87+
text_range: template_chunk_token.text_trimmed_range(),
88+
})
89+
}
90+
_ => None,
91+
}
92+
}
93+
AnyJsExpression::JsStaticMemberExpression(expr) => {
94+
let member = expr.as_fields().member.ok()?;
95+
let value_token = member.as_js_name()?.value_token().ok()?;
96+
let token_text = value_token.text();
97+
is_prototype_builtins(token_text).then_some(RuleState {
98+
prototype_builtins_method_name: token_text.to_string(),
99+
text_range: value_token.text_range(),
100+
})
101+
}
102+
_ => None,
103+
}
104+
}
105+
106+
fn diagnostic(_ctx: &RuleContext<Self>, state: &Self::State) -> Option<RuleDiagnostic> {
107+
let diag = RuleDiagnostic::new(
108+
rule_category!(),
109+
state.text_range,
110+
markup! {
111+
"Do not access Object.prototype method '"{&state.prototype_builtins_method_name}"' from target object."
112+
},
113+
);
114+
115+
if state.prototype_builtins_method_name == "hasOwnProperty" {
116+
Some(
117+
diag.note(markup! {
118+
"It's recommended using "<Emphasis>"Object.hasOwn()"</Emphasis>" instead of using "<Emphasis>"Object.hasOwnProperty()"</Emphasis>"."
119+
})
120+
.note(markup! {
121+
"See "<Hyperlink href="https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/hasOwn">"MDN web docs"</Hyperlink>" for more details."
122+
}),
123+
)
124+
} else {
125+
Some(diag)
126+
}
127+
}
128+
}
129+
130+
/// Chekcks if the `Object.prototype` builtins called directly.
131+
fn is_prototype_builtins(token_text: &str) -> bool {
132+
matches!(
133+
token_text,
134+
"hasOwnProperty" | "isPrototypeOf" | "propertyIsEnumerable"
135+
)
136+
}

‎crates/rome_js_analyze/tests/specs/nursery/noPrototypeBuiltins/invalid.js‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
foo.hasOwnProperty("bar");
2+
foo.isPrototypeOf(bar);
3+
foo.propertyIsEnumerable("bar");
4+
foo.bar.hasOwnProperty("bar");
5+
foo.bar.baz.isPrototypeOf("bar");
6+
foo["hasOwnProperty"]("bar");
7+
foo[`isPrototypeOf`]("bar").baz;
8+
foo?.hasOwnProperty(bar);
9+
(foo?.hasOwnProperty)("bar");
10+
foo?.["hasOwnProperty"]("bar");
11+
(foo?.[`hasOwnProperty`])("bar");

‎crates/rome_js_analyze/tests/specs/nursery/noPrototypeBuiltins/invalid.js.snap‎

Lines changed: 208 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,208 @@
1+
---
2+
source: crates/rome_js_analyze/tests/spec_tests.rs
3+
expression: invalid.js
4+
---
5+
# Input
6+
```js
7+
foo.hasOwnProperty("bar");
8+
foo.isPrototypeOf(bar);
9+
foo.propertyIsEnumerable("bar");
10+
foo.bar.hasOwnProperty("bar");
11+
foo.bar.baz.isPrototypeOf("bar");
12+
foo["hasOwnProperty"]("bar");
13+
foo[`isPrototypeOf`]("bar").baz;
14+
foo?.hasOwnProperty(bar);
15+
(foo?.hasOwnProperty)("bar");
16+
foo?.["hasOwnProperty"]("bar");
17+
(foo?.[`hasOwnProperty`])("bar");
18+
```
19+
20+
# Diagnostics
21+
```
22+
invalid.js:1:5 lint/nursery/noPrototypeBuiltins ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
23+
24+
! Do not access Object.prototype method 'hasOwnProperty' from target object.
25+
26+
> 1 │ foo.hasOwnProperty("bar");
27+
│ ^^^^^^^^^^^^^^
28+
2 │ foo.isPrototypeOf(bar);
29+
3 │ foo.propertyIsEnumerable("bar");
30+
31+
i It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
32+
33+
i See MDN web docs for more details.
34+
35+
36+
```
37+
38+
```
39+
invalid.js:2:5 lint/nursery/noPrototypeBuiltins ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
40+
41+
! Do not access Object.prototype method 'isPrototypeOf' from target object.
42+
43+
1 │ foo.hasOwnProperty("bar");
44+
> 2 │ foo.isPrototypeOf(bar);
45+
│ ^^^^^^^^^^^^^
46+
3 │ foo.propertyIsEnumerable("bar");
47+
4 │ foo.bar.hasOwnProperty("bar");
48+
49+
50+
```
51+
52+
```
53+
invalid.js:3:5 lint/nursery/noPrototypeBuiltins ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
54+
55+
! Do not access Object.prototype method 'propertyIsEnumerable' from target object.
56+
57+
1 │ foo.hasOwnProperty("bar");
58+
2 │ foo.isPrototypeOf(bar);
59+
> 3 │ foo.propertyIsEnumerable("bar");
60+
│ ^^^^^^^^^^^^^^^^^^^^
61+
4 │ foo.bar.hasOwnProperty("bar");
62+
5 │ foo.bar.baz.isPrototypeOf("bar");
63+
64+
65+
```
66+
67+
```
68+
invalid.js:4:9 lint/nursery/noPrototypeBuiltins ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
69+
70+
! Do not access Object.prototype method 'hasOwnProperty' from target object.
71+
72+
2 │ foo.isPrototypeOf(bar);
73+
3 │ foo.propertyIsEnumerable("bar");
74+
> 4 │ foo.bar.hasOwnProperty("bar");
75+
│ ^^^^^^^^^^^^^^
76+
5 │ foo.bar.baz.isPrototypeOf("bar");
77+
6 │ foo["hasOwnProperty"]("bar");
78+
79+
i It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
80+
81+
i See MDN web docs for more details.
82+
83+
84+
```
85+
86+
```
87+
invalid.js:5:13 lint/nursery/noPrototypeBuiltins ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
88+
89+
! Do not access Object.prototype method 'isPrototypeOf' from target object.
90+
91+
3 │ foo.propertyIsEnumerable("bar");
92+
4 │ foo.bar.hasOwnProperty("bar");
93+
> 5 │ foo.bar.baz.isPrototypeOf("bar");
94+
│ ^^^^^^^^^^^^^
95+
6 │ foo["hasOwnProperty"]("bar");
96+
7 │ foo[`isPrototypeOf`]("bar").baz;
97+
98+
99+
```
100+
101+
```
102+
invalid.js:6:5 lint/nursery/noPrototypeBuiltins ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
103+
104+
! Do not access Object.prototype method 'hasOwnProperty' from target object.
105+
106+
4 │ foo.bar.hasOwnProperty("bar");
107+
5 │ foo.bar.baz.isPrototypeOf("bar");
108+
> 6 │ foo["hasOwnProperty"]("bar");
109+
│ ^^^^^^^^^^^^^^^^
110+
7 │ foo[`isPrototypeOf`]("bar").baz;
111+
8 │ foo?.hasOwnProperty(bar);
112+
113+
i It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
114+
115+
i See MDN web docs for more details.
116+
117+
118+
```
119+
120+
```
121+
invalid.js:7:6 lint/nursery/noPrototypeBuiltins ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
122+
123+
! Do not access Object.prototype method 'isPrototypeOf' from target object.
124+
125+
5 │ foo.bar.baz.isPrototypeOf("bar");
126+
6 │ foo["hasOwnProperty"]("bar");
127+
> 7 │ foo[`isPrototypeOf`]("bar").baz;
128+
│ ^^^^^^^^^^^^^
129+
8 │ foo?.hasOwnProperty(bar);
130+
9 │ (foo?.hasOwnProperty)("bar");
131+
132+
133+
```
134+
135+
```
136+
invalid.js:8:6 lint/nursery/noPrototypeBuiltins ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
137+
138+
! Do not access Object.prototype method 'hasOwnProperty' from target object.
139+
140+
6 │ foo["hasOwnProperty"]("bar");
141+
7 │ foo[`isPrototypeOf`]("bar").baz;
142+
> 8 │ foo?.hasOwnProperty(bar);
143+
│ ^^^^^^^^^^^^^^
144+
9 │ (foo?.hasOwnProperty)("bar");
145+
10 │ foo?.["hasOwnProperty"]("bar");
146+
147+
i It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
148+
149+
i See MDN web docs for more details.
150+
151+
152+
```
153+
154+
```
155+
invalid.js:9:7 lint/nursery/noPrototypeBuiltins ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
156+
157+
! Do not access Object.prototype method 'hasOwnProperty' from target object.
158+
159+
7 │ foo[`isPrototypeOf`]("bar").baz;
160+
8 │ foo?.hasOwnProperty(bar);
161+
> 9 │ (foo?.hasOwnProperty)("bar");
162+
│ ^^^^^^^^^^^^^^
163+
10 │ foo?.["hasOwnProperty"]("bar");
164+
11 │ (foo?.[`hasOwnProperty`])("bar");
165+
166+
i It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
167+
168+
i See MDN web docs for more details.
169+
170+
171+
```
172+
173+
```
174+
invalid.js:10:7 lint/nursery/noPrototypeBuiltins ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
175+
176+
! Do not access Object.prototype method 'hasOwnProperty' from target object.
177+
178+
8 │ foo?.hasOwnProperty(bar);
179+
9 │ (foo?.hasOwnProperty)("bar");
180+
> 10 │ foo?.["hasOwnProperty"]("bar");
181+
│ ^^^^^^^^^^^^^^^^
182+
11 │ (foo?.[`hasOwnProperty`])("bar");
183+
184+
i It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
185+
186+
i See MDN web docs for more details.
187+
188+
189+
```
190+
191+
```
192+
invalid.js:11:9 lint/nursery/noPrototypeBuiltins ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
193+
194+
! Do not access Object.prototype method 'hasOwnProperty' from target object.
195+
196+
9 │ (foo?.hasOwnProperty)("bar");
197+
10 │ foo?.["hasOwnProperty"]("bar");
198+
> 11 │ (foo?.[`hasOwnProperty`])("bar");
199+
│ ^^^^^^^^^^^^^^
200+
201+
i It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
202+
203+
i See MDN web docs for more details.
204+
205+
206+
```
207+
208+

0 commit comments

Comments
 (0)