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

Commit

Permalink
feat(rome_js_analyze): noDangerouslySetInnerHtml rule (#3207)
Browse files Browse the repository at this point in the history
  • Loading branch information
ematipico committed Sep 16, 2022
1 parent 11fe745 commit 553f4d6
Show file tree
Hide file tree
Showing 19 changed files with 415 additions and 7 deletions.
5 changes: 5 additions & 0 deletions crates/rome_js_analyze/src/semantic_analyzers/jsx.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion crates/rome_js_analyze/src/semantic_analyzers/nursery.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
use crate::semantic_services::Semantic;
use crate::utils::{is_react_create_element, PossibleCreateElement};
use rome_analyze::context::RuleContext;
use rome_analyze::{declare_rule, Rule, RuleCategory, RuleDiagnostic};
use rome_console::codespan::Severity;
use rome_console::markup;
use rome_js_syntax::{
JsAnyExpression, JsCallExpression, JsLiteralMemberName, JsxAnyAttributeName, JsxAttribute,
};
use rome_rowan::{declare_node_union, AstNode};

declare_rule! {
/// Prevent the usage of dangerous JSX props
///
/// ## Examples
///
/// ### Invalid
///
/// ```jsx,expect_diagnostic
/// function createMarkup() {
/// return { __html: 'child' }
/// }
/// <div dangerouslySetInnerHTML={createMarkup()}></div>
/// ```
///
/// ```js,expect_diagnostic
/// React.createElement('div', {
/// dangerouslySetInnerHTML: { __html: 'child' }
/// });
/// ```
pub(crate) NoDangerouslySetInnerHtml {
version: "0.10.0",
name: "noDangerouslySetInnerHtml",
recommended: false,
}
}

declare_node_union! {
pub(crate) JsAnyCreateElement = JsxAttribute | JsCallExpression
}

pub(crate) enum NoDangerState {
Attribute(JsxAttribute),
Property(JsLiteralMemberName),
}

impl Rule for NoDangerouslySetInnerHtml {
const CATEGORY: RuleCategory = RuleCategory::Lint;

type Query = Semantic<JsAnyCreateElement>;
type State = NoDangerState;
type Signals = Option<Self::State>;

fn run(ctx: &RuleContext<Self>) -> Self::Signals {
let node = ctx.query();
let model = ctx.model();
match node {
JsAnyCreateElement::JsxAttribute(jsx_attribute) => {
let name = jsx_attribute.name().ok()?;
match name {
JsxAnyAttributeName::JsxName(jsx_name) => {
if jsx_name.syntax().text_trimmed() == "dangerouslySetInnerHTML" {
return Some(NoDangerState::Attribute(jsx_attribute.clone()));
}
}
JsxAnyAttributeName::JsxNamespaceName(_) => return None,
}
}
JsAnyCreateElement::JsCallExpression(call_expression) => {
let callee = call_expression.callee().ok()?;
let is_create_element = match callee {
JsAnyExpression::JsStaticMemberExpression(static_member) => {
is_react_create_element(PossibleCreateElement::from(static_member), model)
}
JsAnyExpression::JsIdentifierExpression(identifier_expression) => {
is_react_create_element(
PossibleCreateElement::from(identifier_expression),
model,
)
}
_ => return None,
}?;

if is_create_element {
// if we are inside a create element call, we inspect the second argument, which
// should be an object expression. We look for a member that has as name
// "dangerouslySetInnerHTML"
let arguments = call_expression.arguments().ok()?.args();
let mut arguments = arguments.into_iter();
if let (Some(_), Some(second_argument)) = (arguments.next(), arguments.next()) {
let second_argument = second_argument.ok()?;
let object_expression = second_argument
.as_js_any_expression()?
.as_js_object_expression()?;
let members = object_expression.members();
for member in members {
let member = member.ok()?;
let property_member =
member.as_js_property_object_member()?.name().ok()?;
let name = property_member.as_js_literal_member_name()?;

if name.syntax().text_trimmed() == "dangerouslySetInnerHTML" {
return Some(NoDangerState::Property(name.clone()));
}
}
}
}
}
}

None
}

fn diagnostic(_ctx: &RuleContext<Self>, state: &Self::State) -> Option<RuleDiagnostic> {
let text_range = match state {
NoDangerState::Attribute(jsx_attribute) => {
let name = jsx_attribute.name().ok()?;
name.syntax().text_trimmed_range()
}
NoDangerState::Property(property) => property.syntax().text_trimmed_range(),
};

let diagnostic = RuleDiagnostic::new(
text_range,
markup! {
"Avoid passing content using the "<Emphasis>"dangerouslySetInnerHTML"</Emphasis>" prop."
}
.to_owned(),
).footer(
Severity::Warning,
"Setting content using code can expose users to cross-site scripting (XSS) attacks",
);
Some(diagnostic)
}
}
90 changes: 87 additions & 3 deletions crates/rome_js_analyze/src/utils.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
use rome_js_factory::make;
use rome_js_semantic::SemanticModel;
use rome_js_syntax::JsSyntaxKind::JS_IMPORT;
use rome_js_syntax::{
JsAnyStatement, JsLanguage, JsModuleItemList, JsStatementList, JsVariableDeclaration,
JsVariableDeclarator, JsVariableDeclaratorList, JsVariableStatement, T,
JsAnyStatement, JsIdentifierBinding, JsIdentifierExpression, JsLanguage, JsModuleItemList,
JsStatementList, JsStaticMemberExpression, JsVariableDeclaration, JsVariableDeclarator,
JsVariableDeclaratorList, JsVariableStatement, T,
};
use rome_rowan::{AstNode, AstSeparatedList, BatchMutation};
use rome_rowan::{declare_node_union, AstNode, AstSeparatedList, BatchMutation};
use std::borrow::Cow;

pub mod batch;
Expand Down Expand Up @@ -237,3 +240,84 @@ fn ok_to_camel_case() {
Cow::Owned(s) if s.as_str() == "longCamelCase"
));
}

declare_node_union! {
pub(crate) PossibleCreateElement = JsStaticMemberExpression | JsIdentifierExpression
}

/// Checks if the current node is a possible `createElement` call.
///
/// There are two cases:
///
/// First case
/// ```js
/// React.createElement()
/// ```
/// We check if the node is a static member expression with the specific members. Also, if `React`
/// has been imported in the current scope, we make sure that the binding `React` has been imported
/// from the `"react"` module.
///
/// Second case
///
/// ```js
/// createElement()
/// ```
///
/// The logic of this second case is very similar to the previous one, simply the node that we have
/// to inspect is different.
pub(crate) fn is_react_create_element(
node: PossibleCreateElement,
model: &SemanticModel,
) -> Option<bool> {
let result = match node {
PossibleCreateElement::JsStaticMemberExpression(node) => {
let object = node.object().ok()?;
let member = node.member().ok()?;
let member = member.as_js_name()?;
let identifier = object.as_js_identifier_expression()?.name().ok()?;

let maybe_from_react = identifier.syntax().text_trimmed() == "React"
&& member.syntax().text_trimmed() == "createElement";

if maybe_from_react {
let identifier_binding = model.declaration(&identifier);
if let Some(binding_identifier) = identifier_binding {
let binding_identifier =
JsIdentifierBinding::cast_ref(binding_identifier.syntax())?;
for ancestor in binding_identifier.syntax().ancestors() {
if ancestor.kind() == JS_IMPORT {
return Some(
binding_identifier.syntax().text_trimmed()
== identifier.syntax().text_trimmed(),
);
}
}
}
}
maybe_from_react
}
PossibleCreateElement::JsIdentifierExpression(identifier) => {
let name = identifier.name().ok()?;
let maybe_from_react = identifier.syntax().text_trimmed() == "createElement";
if maybe_from_react {
let identifier_binding = model.declaration(&name);
if let Some(identifier_binding) = identifier_binding {
let binding_identifier =
JsIdentifierBinding::cast_ref(identifier_binding.syntax())?;
for ancestor in binding_identifier.syntax().ancestors() {
if ancestor.kind() == JS_IMPORT {
return Some(
binding_identifier.syntax().text_trimmed()
== identifier.syntax().text_trimmed(),
);
}
}
}
}

maybe_from_react
}
};

Some(result)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import React, { createElement } from "react";

React.createElement('div', {
dangerouslySetInnerHTML: { __html: 'child' }
});

createElement('div', {
dangerouslySetInnerHTML: { __html: 'child' }
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
---
source: crates/rome_js_analyze/tests/spec_tests.rs
expression: createElementBinding.js
---
# Input
```js
import React, { createElement } from "react";
React.createElement('div', {
dangerouslySetInnerHTML: { __html: 'child' }
});
createElement('div', {
dangerouslySetInnerHTML: { __html: 'child' }
});
```

# Diagnostics
```
warning[nursery/noDangerouslySetInnerHtml]: Avoid passing content using the dangerouslySetInnerHTML prop.
┌─ createElementBinding.js:4:5
4 │ dangerouslySetInnerHTML: { __html: 'child' }
│ -----------------------
= warning: Setting content using code can expose users to cross-site scripting (XSS) attacks
```

```
warning[nursery/noDangerouslySetInnerHtml]: Avoid passing content using the dangerouslySetInnerHTML prop.
┌─ createElementBinding.js:8:5
8 │ dangerouslySetInnerHTML: { __html: 'child' }
│ -----------------------
= warning: Setting content using code can expose users to cross-site scripting (XSS) attacks
```


Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// invalid
let a = <div dangerouslySetInnerHTML={{ __html: 'child' }} />

// valid
let b = <div foo="" />
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
---
source: crates/rome_js_analyze/tests/spec_tests.rs
expression: insideJsx.jsx
---
# Input
```js
// invalid
let a = <div dangerouslySetInnerHTML={{ __html: 'child' }} />
// valid
let b = <div foo="" />
```

# Diagnostics
```
warning[nursery/noDangerouslySetInnerHtml]: Avoid passing content using the dangerouslySetInnerHTML prop.
┌─ insideJsx.jsx:2:14
2 │ let a = <div dangerouslySetInnerHTML={{ __html: 'child' }} />
│ -----------------------
= warning: Setting content using code can expose users to cross-site scripting (XSS) attacks
```


Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
React.createElement('div', {
dangerouslySetInnerHTML: { __html: 'child' }
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
---
source: crates/rome_js_analyze/tests/spec_tests.rs
expression: reactCreateElement.js
---
# Input
```js
React.createElement('div', {
dangerouslySetInnerHTML: { __html: 'child' }
});
```

# Diagnostics
```
warning[nursery/noDangerouslySetInnerHtml]: Avoid passing content using the dangerouslySetInnerHTML prop.
┌─ reactCreateElement.js:2:5
2 │ dangerouslySetInnerHTML: { __html: 'child' }
│ -----------------------
= warning: Setting content using code can expose users to cross-site scripting (XSS) attacks
```


1 change: 1 addition & 0 deletions crates/rome_js_semantic/src/semantic_model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,7 @@ enum ReferenceType {
}

/// Provides all information regarding to a specific reference.
#[derive(Debug)]
pub struct Reference {
data: Arc<SemanticModelData>,
node: JsSyntaxNode,
Expand Down
4 changes: 3 additions & 1 deletion crates/rome_service/src/configuration/linter/rules.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 553f4d6

Please sign in to comment.