From 1f770087316617106f9bbc689a0ca76f49c7136c Mon Sep 17 00:00:00 2001 From: Ed Sanders Date: Thu, 20 Jun 2024 14:52:54 +0100 Subject: [PATCH] Rule fix: `no-append-html`: Allow passing selectors to some methods Fixes #326 --- docs/rules/no-append-html.md | 9 ++++++++- src/rules/no-append-html.js | 15 ++++++++++++--- src/utils.js | 2 ++ tests/rules/no-append-html.js | 9 +++++++++ 4 files changed, 31 insertions(+), 4 deletions(-) diff --git a/docs/rules/no-append-html.md b/docs/rules/no-append-html.md index 3a9564f..3de73e9 100644 --- a/docs/rules/no-append-html.md +++ b/docs/rules/no-append-html.md @@ -2,13 +2,14 @@ # no-append-html -Disallows using [`.append`](https://api.jquery.com/append/)/[`.prepend`](https://api.jquery.com/prepend/)/[`.before`](https://api.jquery.com/before/)/[`.after`](https://api.jquery.com/after/)/[`.replaceWith`](https://api.jquery.com/replaceWith/)/[`.add`](https://api.jquery.com/add/)/[`.appendTo`](https://api.jquery.com/appendTo/)/[`.prependTo`](https://api.jquery.com/prependTo/) to inject HTML, in order to prevent possible XSS bugs. +Disallows using [`.append`](https://api.jquery.com/append/)/[`.prepend`](https://api.jquery.com/prepend/)/[`.before`](https://api.jquery.com/before/)/[`.after`](https://api.jquery.com/after/)/[`.replaceWith`](https://api.jquery.com/replaceWith/)/[`.add`](https://api.jquery.com/add/)/[`.appendTo`](https://api.jquery.com/appendTo/)/[`.prependTo`](https://api.jquery.com/prependTo/)/[`.insertBefore`](https://api.jquery.com/insertBefore/)/[`.insertAfter`](https://api.jquery.com/insertAfter/) to inject HTML, in order to prevent possible XSS bugs. ## Rule details ❌ Examples of **incorrect** code: ```js $div.append( '' ); +$div.append( 'unescaped html' ); $div.prepend( '' ); $div.before( '' ); $div.after( '' ); @@ -47,6 +48,12 @@ $div.append( test ? $el1 : '' ); $el = getSomething(); $div.append( $el ); + +$div.add( '.foo' ); +$div.appendTo( '.foo' ); +$div.prependTo( '.foo' ); +$div.insertBefore( '.foo' ); +$div.insertAfter( '.foo' ); ``` ## Resources diff --git a/src/rules/no-append-html.js b/src/rules/no-append-html.js index 1942c5f..7e778e0 100644 --- a/src/rules/no-append-html.js +++ b/src/rules/no-append-html.js @@ -1,7 +1,11 @@ 'use strict'; const utils = require( '../utils.js' ); -const methods = [ 'append', 'prepend', 'before', 'after', 'replaceWith', 'add', 'appendTo', 'prependTo' ]; +// htmlStrings or jQuery collections +const htmlOrCollectionMethods = [ 'append', 'prepend', 'before', 'after', 'replaceWith' ]; +// htmlStrings, selectors or jQuery collections +const htmlOrSelectorOrCollectionMethods = [ 'add', 'appendTo', 'prependTo', 'insertBefore', 'insertAfter' ]; +const allMethods = htmlOrCollectionMethods.concat( htmlOrSelectorOrCollectionMethods ); function alljQueryOrEmpty( context, node ) { if ( node.type === 'ConditionalExpression' ) { @@ -22,7 +26,7 @@ module.exports = { meta: { type: 'suggestion', docs: { - description: 'Disallows using ' + methods.map( utils.jQueryCollectionLink ).join( '/' ) + + description: 'Disallows using ' + allMethods.map( utils.jQueryCollectionLink ).join( '/' ) + ' to inject HTML, in order to prevent possible XSS bugs.' }, schema: [] @@ -32,13 +36,18 @@ module.exports = { 'CallExpression:exit': ( node ) => { if ( !( node.callee.type === 'MemberExpression' && - methods.includes( node.callee.property.name ) + allMethods.includes( node.callee.property.name ) ) ) { return; } if ( node.arguments.every( ( arg ) => alljQueryOrEmpty( context, arg ) ) ) { return; } + if ( htmlOrSelectorOrCollectionMethods.includes( node.callee.property.name ) ) { + if ( node.arguments.every( ( arg ) => !utils.isHtmlString( arg ) ) ) { + return; + } + } if ( utils.isjQuery( context, node.callee ) ) { context.report( { diff --git a/src/utils.js b/src/utils.js index 1ae1d07..066a092 100644 --- a/src/utils.js +++ b/src/utils.js @@ -544,6 +544,8 @@ function eventShorthandFixer( node, context, fixer ) { function allLiteral( node ) { if ( node.type === 'BinaryExpression' ) { return allLiteral( node.left ) && allLiteral( node.right ); + } else if ( node.type === 'ConditionalExpression' ) { + return allLiteral( node.consequent ) && allLiteral( node.alternate ); } else { return node.type === 'Literal'; } diff --git a/tests/rules/no-append-html.js b/tests/rules/no-append-html.js index ba447f5..4d1ed61 100644 --- a/tests/rules/no-append-html.js +++ b/tests/rules/no-append-html.js @@ -27,6 +27,11 @@ ruleTester.run( 'no-append-html', rule, { '$div.append(test ? $el1 : undefined)', '$div.append(test ? $el1 : "")', '$el=getSomething();\n$div.append($el);', + '$div.add(".foo")', + '$div.appendTo(".foo")', + '$div.prependTo(".foo")', + '$div.insertBefore(".foo")', + '$div.insertAfter(".foo")', { code: 'div.append("")', docgen: false @@ -37,6 +42,10 @@ ruleTester.run( 'no-append-html', rule, { code: '$div.append("")', errors: [ error ] }, + { + code: '$div.append("unescaped html")', + errors: [ error ] + }, { code: '$div.prepend("")', errors: [ error ]