From 34a219037e561aef3e258d00c477f720253bde29 Mon Sep 17 00:00:00 2001 From: yassin-kammoun-sonarsource Date: Wed, 5 Jun 2024 11:21:01 +0200 Subject: [PATCH] JS-164 Improve S5122: Detect origin reflection --- .../jsts/TypeScript/typescript-S5122.json | 5 - .../expected/jsts/p5.js/javascript-S5122.json | 7 - .../jsts/postgraphql/javascript-S5122.json | 9 - .../expected/jsts/yaml/javascript-S5122.json | 10 - .../jsts/src/rules/S5122/cb.cors.fixture.js | 30 ++ .../jsts/src/rules/S5122/cb.http.fixture.js | 52 +++ .../src/rules/S5122/cb.reproducer.fixture.js | 23 ++ .../src/rules/S5122/cb.tainted.fixture.js | 79 ++++ packages/jsts/src/rules/S5122/cb.test.ts | 28 ++ packages/jsts/src/rules/S5122/rule.ts | 366 +++++++++++++++--- packages/jsts/src/rules/S5122/unit.test.ts | 283 -------------- .../javascript/rules/javascript/S5122.html | 6 +- 12 files changed, 520 insertions(+), 378 deletions(-) delete mode 100644 its/ruling/src/test/expected/jsts/TypeScript/typescript-S5122.json delete mode 100644 its/ruling/src/test/expected/jsts/p5.js/javascript-S5122.json delete mode 100644 its/ruling/src/test/expected/jsts/postgraphql/javascript-S5122.json delete mode 100644 its/ruling/src/test/expected/jsts/yaml/javascript-S5122.json create mode 100644 packages/jsts/src/rules/S5122/cb.cors.fixture.js create mode 100644 packages/jsts/src/rules/S5122/cb.http.fixture.js create mode 100644 packages/jsts/src/rules/S5122/cb.reproducer.fixture.js create mode 100644 packages/jsts/src/rules/S5122/cb.tainted.fixture.js create mode 100644 packages/jsts/src/rules/S5122/cb.test.ts delete mode 100644 packages/jsts/src/rules/S5122/unit.test.ts diff --git a/its/ruling/src/test/expected/jsts/TypeScript/typescript-S5122.json b/its/ruling/src/test/expected/jsts/TypeScript/typescript-S5122.json deleted file mode 100644 index e5a86ed6543..00000000000 --- a/its/ruling/src/test/expected/jsts/TypeScript/typescript-S5122.json +++ /dev/null @@ -1,5 +0,0 @@ -{ -"TypeScript:src/harness/harness.ts": [ -643 -] -} diff --git a/its/ruling/src/test/expected/jsts/p5.js/javascript-S5122.json b/its/ruling/src/test/expected/jsts/p5.js/javascript-S5122.json deleted file mode 100644 index ba975d2b03a..00000000000 --- a/its/ruling/src/test/expected/jsts/p5.js/javascript-S5122.json +++ /dev/null @@ -1,7 +0,0 @@ -{ -"p5.js:Gruntfile.js": [ -340, -367, -394 -] -} diff --git a/its/ruling/src/test/expected/jsts/postgraphql/javascript-S5122.json b/its/ruling/src/test/expected/jsts/postgraphql/javascript-S5122.json deleted file mode 100644 index f5b1d2d7e5e..00000000000 --- a/its/ruling/src/test/expected/jsts/postgraphql/javascript-S5122.json +++ /dev/null @@ -1,9 +0,0 @@ -{ -"postgraphql:src/postgraphql/http/__tests__/createPostGraphQLHttpRequestHandler-test.js": [ -139, -152 -], -"postgraphql:src/postgraphql/http/createPostGraphQLHttpRequestHandler.js": [ -504 -] -} diff --git a/its/ruling/src/test/expected/jsts/yaml/javascript-S5122.json b/its/ruling/src/test/expected/jsts/yaml/javascript-S5122.json deleted file mode 100644 index e250b3eefb3..00000000000 --- a/its/ruling/src/test/expected/jsts/yaml/javascript-S5122.json +++ /dev/null @@ -1,10 +0,0 @@ -{ -"yaml:lambda-block-folded-2.yaml": [ -131, -181, -219 -], -"yaml:lambda-block-literal.yaml": [ -463 -] -} diff --git a/packages/jsts/src/rules/S5122/cb.cors.fixture.js b/packages/jsts/src/rules/S5122/cb.cors.fixture.js new file mode 100644 index 00000000000..8fd7bd0b35f --- /dev/null +++ b/packages/jsts/src/rules/S5122/cb.cors.fixture.js @@ -0,0 +1,30 @@ +const express = require('express'); +const cors = require('cors'); + +const app1 = express(); +app1.use(cors()); // Noncompliant + +const app2 = express(); +app2.use(cors({})); // Noncompliant + +const app3 = express(); +app3.use(cors({ origin: '*' })); // Noncompliant + +const app4 = express(); +const corsOptions = { origin: '*' }; +// ^^^^^^^^^^^> {{Sensitive configuration}} + app4.use(cors(corsOptions)); // Noncompliant {{Make sure that enabling CORS is safe here.}} +//^^^^^^^^ + +const app5 = express(); +const corsOpts = { origin: '*' }; +const corsHandler = cors(corsOpts); +app5.use(corsHandler); // Noncompliant + +const app42 = express(); +app42.use(); +app42.use(undefined); +app42.use('cors'); +app42.use(foo()); +app42.use(cors(42)); +app42.use(cors({ origin: 'value' })); diff --git a/packages/jsts/src/rules/S5122/cb.http.fixture.js b/packages/jsts/src/rules/S5122/cb.http.fixture.js new file mode 100644 index 00000000000..84739bf058d --- /dev/null +++ b/packages/jsts/src/rules/S5122/cb.http.fixture.js @@ -0,0 +1,52 @@ +const http = require('http'); + +function listener(req, res) { + res.writeHead(200, { 'Access-Control-Allow-Origin': '*' }); // Noncompliant + res.end('ok'); +} + +http.createServer(listener); + +http.createServer((_, res) => { + res.writeHead(200, { 'Access-Control-Allow-Origin': '*' }); // Noncompliant + res.end('ok'); +}); + +http.createServer((_, res) => { + const header = { 'Access-Control-Allow-Origin': '*' }; +// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^> {{Sensitive configuration}} + res.writeHead(200, header); // Noncompliant {{Make sure that enabling CORS is safe here.}} +//^^^^^^^^^^^^^ + res.end('ok'); +}); + +http.createServer((_, res) => { + const access = '*'; + const header = { 'Access-Control-Allow-Origin': access }; + res.writeHead(200, header); // Noncompliant + res.end('ok'); +}); + +http.createServer(); +http.createServer(undefined); +http.createServer('listener'); +http.createServer(_ => {}); +http.createServer((req, res) => {}); +http.createServer((_, res) => { + res.end('ok'); +}); +http.createServer((_, res) => { + res.writeHead(200, 'header'); +}); +http.createServer((_, res) => { + res.writeHead(200, {}); +}); +http.createServer((_, res) => { + res.writeHead(200, { undefined }); +}); +http.createServer((_, res) => { + res.writeHead(200, { header: 'value' }); +}); +http.createServer((_, res) => { + res.writeHead(200, { 'Access-Control-Allow-Origin': 'value' }); +}); diff --git a/packages/jsts/src/rules/S5122/cb.reproducer.fixture.js b/packages/jsts/src/rules/S5122/cb.reproducer.fixture.js new file mode 100644 index 00000000000..677e2559305 --- /dev/null +++ b/packages/jsts/src/rules/S5122/cb.reproducer.fixture.js @@ -0,0 +1,23 @@ +// Code reproducer from R&D +var express = require('express'); +var app = express(); + +function cgiHandler(req, res) { + if (UP_PATH_REGEXP.test(req.path)) { + return res.status(403).end('Forbidden'); + } + if (req.headers.origin) { + res.setHeader('access-control-allow-origin', req.headers.origin); // Noncompliant + res.setHeader('access-control-allow-credentials', true); + } + // ... +} + +app.all('/cgi-bin/sessions/*', cgiHandler); + +app.all('/cgi-bin/*', function(req, res, next) { + req.isUploadReq = UPLOAD_URLS.indexOf(req.path) !== -1; + return req.isUploadReq ? uploadUrlencodedParser(req, res, next) : urlencodedParser(req, res, next); +}, function(req, res, next) { + return req.isUploadReq ? uploadJsonParser(req, res, next) : jsonParser(req, res, next); +}, cgiHandler); diff --git a/packages/jsts/src/rules/S5122/cb.tainted.fixture.js b/packages/jsts/src/rules/S5122/cb.tainted.fixture.js new file mode 100644 index 00000000000..c51c23c0956 --- /dev/null +++ b/packages/jsts/src/rules/S5122/cb.tainted.fixture.js @@ -0,0 +1,79 @@ +const express = require('express'); +const app = express(); + +app.all('*', (req, res) => { + res.setHeader('access-control-allow-origin', req.headers.origin); // Noncompliant +}); + +app.all('*', (req, res) => { + res.setHeader('access-control-allow-origin', req.header('origin')); // Noncompliant +}); + +app.all('*', (req, res) => { + const origin = req.headers.origin; +// ^^^^^^^^^^^^^^^^^^> {{Sensitive configuration}} + res.setHeader('access-control-allow-origin', origin); // Noncompliant {{Make sure that enabling CORS is safe here.}} +//^^^^^^^^^^^^^ +}); + +app.all('*', (req, res) => { + const origin = req.header('origin'); + res.setHeader('access-control-allow-origin', origin); // Noncompliant +}); + +app.all('*', (req, res) => { + res.setHeader('access-control-allow-origin', sanitize(req.headers.origin)); +}); + +app.all('*', (req, res) => { + res.setHeader('access-control-allow-origin', sanitize(req.header('origin'))); +}); + +app.all('*', (req, res) => { + const origin = req.headers.origin; + const sanitized = sanitize(origin); + res.setHeader('access-control-allow-origin', sanitized); +}); + +app.all('*', (req, res) => { + const origin = req.header('Origin'); + const sanitized = sanitize(origin); + res.setHeader('access-control-allow-origin', sanitized); +}); + +app.all('*', (req, res) => { + const origin = req.headers.origin; + if (origin === 'https://www.trusted.com') { + res.setHeader('access-control-allow-origin', origin); + } +}); + +app.all('*', (req, res) => { + const origin = req.header('origin'); + if (origin === 'https://www.trusted.com') { + res.setHeader('access-control-allow-origin', origin); + } +}); + +app.all('*'); +app.all('*', 42); +app.all('*', undefined); +app.all('*', ({})); +app.all('*', () => {}); +app.all('*', req => {}); +app.all('*', ({}) => {}); +app.all('*', (req, res) => {}); +app.all('*', (req, {}) => {}); +app.all('*', (req, res) => { res; }); +app.all('*', (req, res) => { res.end('ok'); }); +app.all('*', (req, res) => { res.setHeader(); }); +app.all('*', (req, res) => { res.setHeader(42); }); +app.all('*', (req, res) => { res.setHeader('foo'); }); +app.all('*', (req, res) => { res.setHeader('access-control-allow-origin'); }); +app.all('*', (req, res) => { res.setHeader('access-control-allow-origin', undefined); }); +app.all('*', (req, res) => { res.setHeader('access-control-allow-origin', foo()); }); +app.all('*', (req, res) => { res.setHeader('access-control-allow-origin', req.header()); }); +app.all('*', (req, res) => { res.setHeader('access-control-allow-origin', req.header(42)); }); +app.all('*', (req, res) => { res.setHeader('access-control-allow-origin', req.header('foo')); }); +app.all('*', (req, res) => { res.setHeader('access-control-allow-origin', req.headers); }); +app.all('*', (req, res) => { res.setHeader('access-control-allow-origin', req.headers.foo); }); diff --git a/packages/jsts/src/rules/S5122/cb.test.ts b/packages/jsts/src/rules/S5122/cb.test.ts new file mode 100644 index 00000000000..b634ae8cb12 --- /dev/null +++ b/packages/jsts/src/rules/S5122/cb.test.ts @@ -0,0 +1,28 @@ +/* + * SonarQube JavaScript Plugin + * Copyright (C) 2011-2024 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +import { check } from '../tools'; +import { rule } from './'; +import path from 'path'; + +const sonarId = path.basename(__dirname); + +describe('Rule S5122', () => { + check(sonarId, rule, __dirname); +}); diff --git a/packages/jsts/src/rules/S5122/rule.ts b/packages/jsts/src/rules/S5122/rule.ts index 87e6016f8dd..7d0d71c7d6e 100644 --- a/packages/jsts/src/rules/S5122/rule.ts +++ b/packages/jsts/src/rules/S5122/rule.ts @@ -22,17 +22,48 @@ import { Rule } from 'eslint'; import * as estree from 'estree'; import { - getUniqueWriteUsage, - toEncodedMessage, + findFirstMatchingLocalAncestor, getFullyQualifiedName, getProperty, + getUniqueWriteUsageOrNode, + getValueOfExpression, + isCallingMethod, + isIdentifier, + resolveFunction, + toEncodedMessage, } from '../helpers'; import { TSESTree } from '@typescript-eslint/utils'; import { SONAR_RUNTIME } from '../../linter/parameters'; const MESSAGE = `Make sure that enabling CORS is safe here.`; - -const CORS_HEADER = 'Access-Control-Allow-Origin'; +const SECONDARY_MESSAGE = 'Sensitive configuration'; +const ACCESS_CONTROL_ALLOW_ORIGIN = 'Access-Control-Allow-Origin'; +const ROUTINE_METHODS = new Set([ + 'all', // 'all' is a special method that matches any HTTP method + 'checkout', + 'copy', + 'delete', + 'get', + 'head', + 'lock', + 'merge', + 'mkactivity', + 'mkcol', + 'move', + 'm-search', + 'notify', + 'options', + 'patch', + 'post', + 'purge', + 'put', + 'report', + 'search', + 'subscribe', + 'trace', + 'unlock', + 'unsubscribe', +]); export const rule: Rule.RuleModule = { meta: { @@ -44,74 +75,287 @@ export const rule: Rule.RuleModule = { ], }, create(context: Rule.RuleContext) { - function report(node: estree.Node, ...secondaryLocations: estree.Node[]) { - const message = toEncodedMessage(MESSAGE, secondaryLocations as TSESTree.Node[]); - context.report({ message, node }); + return { + CallExpression(node: estree.CallExpression) { + checkNodeHttp(context, node); + checkExpressCors(context, node); + checkExpressUserControlledOrigin(context, node); + }, + }; + }, +}; + +/** + * Checks that the callback of http.createServer() does not set the 'Access-Control-Allow-Origin' header to '*'. + */ +function checkNodeHttp(context: Rule.RuleContext, node: estree.CallExpression) { + // Check if the node is a call to the 'http.createServer' method + const fqn = getFullyQualifiedName(context, node.callee); + if (fqn !== 'http.createServer') { + return; + } + + // Check if the first argument is defined + const arg0 = node.arguments[0]; + if (!arg0) { + return; + } + + // Check if the first argument is a callback function + const callback = resolveFunction(context, arg0); + if (!callback) { + return; + } + + // Check if the callback has a response parameter + const resParameter = callback.params[1]; + if (!resParameter || !isIdentifier(resParameter)) { + return; + } + + // Retrieve the variable declaration of the response parameter + const resVariable = context.sourceCode.scopeManager + .getDeclaredVariables(callback) + .find(v => v.name === resParameter.name); + if (!resVariable) { + return; + } + + // Check if the response parameter is used to set the 'Access-Control-Allow-Origin' header + const resUsages = resVariable.references; + for (const resUsage of resUsages) { + const identifier = resUsage.identifier as TSESTree.Identifier; + + // Find the first ancestor that is a call to the 'writeHead' method + const writeHeadCall = identifier?.parent?.parent as estree.Node | undefined; + if ( + !(writeHeadCall?.type === 'CallExpression' && isCallingMethod(writeHeadCall, 2, 'writeHead')) + ) { + continue; } - function isCorsCall(call: estree.CallExpression) { - return getFullyQualifiedName(context, call) === 'cors'; + // Check if the header argument of the 'writeHead' method is defined + const headerValue = getValueOfExpression( + context, + writeHeadCall.arguments[1], + 'ObjectExpression', + ); + if (!headerValue) { + continue; } - return { - CallExpression(node: estree.Node) { - const call = node as estree.CallExpression; + // Check if the header argument contains the 'Access-Control-Allow-Origin' property + const accessProperty = getProperty(headerValue, ACCESS_CONTROL_ALLOW_ORIGIN, context); + if (!accessProperty) { + continue; + } - if (isCorsCall(call)) { - if (call.arguments.length === 0) { - report(call); - return; - } - const [arg] = call.arguments; - let sensitiveCorsProperty = getSensitiveCorsProperty(arg, context); - if (sensitiveCorsProperty) { - report(sensitiveCorsProperty); - } - if (arg?.type === 'Identifier') { - const usage = getUniqueWriteUsage(context, arg.name, arg); - sensitiveCorsProperty = getSensitiveCorsProperty(usage, context); - if (sensitiveCorsProperty) { - report(sensitiveCorsProperty, arg); - } - } - } + // Check if the 'Access-Control-Allow-Origin' property is set to '*' + const accessValue = getValueOfExpression(context, accessProperty.value, 'Literal'); + if (accessValue?.value === '*') { + context.report({ + node: writeHeadCall.callee, + message: toEncodedMessage(MESSAGE, [accessProperty], [SECONDARY_MESSAGE]), + }); + } + } +} - if (isSettingCorsHeader(call)) { - report(call); - } - }, +/** + * Checks that the callback of express.use() does not use cors() middleware with sensitive configuration. + */ +function checkExpressCors(context: Rule.RuleContext, node: estree.CallExpression) { + // Check if the node is a call to the 'express.use' method + const fqn = getFullyQualifiedName(context, node.callee); + if (fqn !== 'express.use') { + return; + } - ObjectExpression(node: estree.Node) { - const objProperty = getProperty(node, CORS_HEADER, context); - if (objProperty && isAnyDomain(objProperty.value)) { - report(objProperty); - } - }, - }; - }, -}; + // Check if the first argument is defined + const argValue = getValueOfExpression(context, node.arguments[0], 'CallExpression'); + if (!argValue) { + return; + } -function isCorsHeader(node: estree.Node) { - const header = node as TSESTree.Node; - return header && header.type === 'Literal' && header.value === CORS_HEADER; -} + // Check if the first argument is a call to the 'cors' method + const argFqn = getFullyQualifiedName(context, argValue); + if (argFqn !== 'cors') { + return; + } -function isAnyDomain(node: estree.Node) { - const domain = node as TSESTree.Node; - return domain && domain.type === 'Literal' && domain.value === '*'; -} + // Check if the call to the 'cors' method has an argument (default configuration is sensitive) + const corsOptions = argValue.arguments[0]; + if (!corsOptions) { + context.report({ + node, + message: toEncodedMessage(MESSAGE, [], []), + }); + return; + } + + // Check if the argument of the 'cors' method is an object + const corsOptionsValue = getValueOfExpression(context, corsOptions, 'ObjectExpression'); + if (!corsOptionsValue) { + return; + } + + // Check if the 'origin' property is defined (default configuration is sensitive) + const originProperty = getProperty(corsOptionsValue, 'origin', context); + if (!originProperty) { + context.report({ + node: node.callee, + message: toEncodedMessage(MESSAGE, [corsOptions], [SECONDARY_MESSAGE]), + }); + return; + } -function getSensitiveCorsProperty( - node: estree.Node | undefined | null, - context: Rule.RuleContext, -): estree.Property | undefined { - const originProperty = getProperty(node, 'origin', context); - if (originProperty && isAnyDomain(originProperty.value)) { - return originProperty; + // Check if the 'origin' property is set to '*' + const originValue = getValueOfExpression(context, originProperty.value, 'Literal'); + if (originValue?.value === '*') { + context.report({ + node: node.callee, + message: toEncodedMessage(MESSAGE, [originProperty], [SECONDARY_MESSAGE]), + }); } - return undefined; } -function isSettingCorsHeader(call: estree.CallExpression) { - return isCorsHeader(call.arguments[0]) && isAnyDomain(call.arguments[1]); +/** + * Checks that the callback of express.() does not set the 'Access-Control-Allow-Origin' header to '*'. + */ +function checkExpressUserControlledOrigin(context: Rule.RuleContext, node: estree.CallExpression) { + // Check if the node is a call to an express routine method + const fqn = getFullyQualifiedName(context, node.callee)?.split('.'); + if (!(fqn?.length === 2 && fqn[0] === 'express' && ROUTINE_METHODS.has(fqn[1]))) { + return; + } + + // Check if the second argument is defined + const arg1 = node.arguments[1]; + if (!arg1) { + return; + } + + // Check if the second argument is a callback function + const callback = resolveFunction(context, arg1); + if (!callback) { + return; + } + + // Check if the callback has a request parameter + const reqParameter = callback.params[0]; + if (!reqParameter || !isIdentifier(reqParameter)) { + return; + } + + // Check if the callback has a response parameter + const resParameter = callback.params[1]; + if (!resParameter || !isIdentifier(resParameter)) { + return; + } + + // Retrieve the variable declaration of the response parameter + const resVariable = context.sourceCode.scopeManager + .getDeclaredVariables(callback) + .find(v => v.name === resParameter.name); + if (!resVariable) { + return; + } + + // Check if the response parameter is used to set the 'Access-Control-Allow-Origin' header + const resUsages = resVariable.references; + for (const resUsage of resUsages) { + const identifier = resUsage.identifier as TSESTree.Identifier; + + // Find the first ancestor that is a call to the 'setHeader' method + const setHeaderCall = identifier?.parent?.parent as estree.Node | undefined; + if ( + !(setHeaderCall?.type === 'CallExpression' && isCallingMethod(setHeaderCall, 2, 'setHeader')) + ) { + continue; + } + + // Check if the first argument of the 'setHeader' method is 'Access-Control-Allow-Origin' + const headerName = getValueOfExpression(context, setHeaderCall.arguments[0], 'Literal'); + if ( + !headerName || + typeof headerName.value !== 'string' || + headerName.value.toLowerCase() !== ACCESS_CONTROL_ALLOW_ORIGIN.toLowerCase() + ) { + continue; + } + + // Retrieve the value of the header argument + const headerArg = setHeaderCall.arguments[1]; + const headerValue = getUniqueWriteUsageOrNode(context, headerArg); + + // Check if the header value is a user-controlled origin header + switch (headerValue.type) { + // Check if the header value is a call to the 'req.header' method with 'origin' as argument + case 'CallExpression': { + const callee = headerValue.callee; + const calleeText = context.sourceCode.getText(callee); + if (calleeText === `${reqParameter.name}.header`) { + const originValue = getValueOfExpression(context, headerValue.arguments[0], 'Literal'); + if ( + typeof originValue?.value === 'string' && + originValue.value.toLowerCase() === 'origin' && + !isValidated(callback, headerArg) + ) { + context.report({ + node: setHeaderCall.callee, + message: toEncodedMessage(MESSAGE, [headerValue], [SECONDARY_MESSAGE]), + }); + } + } + break; + } + // Check if the header value is a member expression with 'req.headers.origin' as argument + case 'MemberExpression': { + const headerValueText = context.sourceCode.getText(headerValue); + if ( + headerValueText === `${reqParameter.name}.headers.origin` && + !isValidated(callback, headerArg) + ) { + context.report({ + node: setHeaderCall.callee, + message: toEncodedMessage(MESSAGE, [headerValue], [SECONDARY_MESSAGE]), + }); + } + break; + } + } + } + + /** + * Checks if the user-controlled origin header is validated in the callback through a comparison. + */ + function isValidated(callback: estree.Function, header: estree.Node) { + if (!isIdentifier(header)) { + return false; + } + + // Find the variable declaration of the header + const headerVariable = context.sourceCode.scopeManager + .acquire(callback) + ?.variables.find(v => v.name === header.name); + if (!headerVariable) { + return false; + } + + // Check if the header is compared in a binary expression + const headerUsages = headerVariable.references; + for (const headerUsage of headerUsages) { + const identifier = headerUsage.identifier as TSESTree.Identifier; + const validated = findFirstMatchingLocalAncestor( + identifier, + node => + node.type === 'BinaryExpression' && ['==', '!=', '===', '!=='].includes(node.operator), + ); + if (validated) { + return true; + } + } + + return false; + } } diff --git a/packages/jsts/src/rules/S5122/unit.test.ts b/packages/jsts/src/rules/S5122/unit.test.ts deleted file mode 100644 index 451ea52f9bd..00000000000 --- a/packages/jsts/src/rules/S5122/unit.test.ts +++ /dev/null @@ -1,283 +0,0 @@ -/* - * SonarQube JavaScript Plugin - * Copyright (C) 2011-2024 SonarSource SA - * mailto:info AT sonarsource DOT com - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 3 of the License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public License - * along with this program; if not, write to the Free Software Foundation, - * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. - */ -import { RuleTester } from 'eslint'; -import { rule } from './'; - -const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 2018, sourceType: 'module' } }); - -const EXPECTED_MESSAGE = 'Make sure that enabling CORS is safe here.'; - -const EXPECTED_MESSAGE_WITHOUT_SECONDARY_LOC = JSON.stringify({ - message: EXPECTED_MESSAGE, - secondaryLocations: [], -}); - -ruleTester.run('Enabling Cross-Origin Resource Sharing is security-sensitive', rule, { - valid: [ - { - code: `import * as express from 'foo'; app.use(cors());`, - }, - { - code: `import * as express from 'express'; app.use(bodyParser());`, - }, - { - code: `res.writeHead(200, { 'Content-Type': 'text/html' });`, - }, - { - code: ` - const srv2 = http.createServer((req, res) => { - res.setHeader('Access-Control-Allow-Origin', 'http://mytrustedorigin'); - }); - `, - }, - { - code: ` - res.header('Access-Control-Allow-Origin', 'http://localhost'); - res.set('Access-Control-Max-Age', '86500'); - res.header('Access-Control-Allow-Methods', 'GET,PUT,POST,DELETE'); - res.append('Access-Control-Allow-Credentials', 'true'); - `, - }, - { - code: ` - const express = require('express'); - const cors = require('cors'); - app.use(cors({ - origin: 'http://localhost', // Compliant - optionsSuccessStatus: 200 - })); - app.use(cors(foo())); - `, - }, - { - code: ` - const express = require('express'); - app.use(require('cors')({ origin: 'http://localhost' })); // Compliant - `, - }, - ], - invalid: [ - { - code: `res.writeHead(200, { 'Access-Control-Allow-Origin': '*' });`, - errors: [ - { - message: EXPECTED_MESSAGE_WITHOUT_SECONDARY_LOC, - line: 1, - endLine: 1, - column: 22, - endColumn: 56, - }, - ], - }, - { - code: ` - import * as express from 'express'; - import * as cors from 'cors'; - app.use(cors());`, - errors: [ - { - message: EXPECTED_MESSAGE_WITHOUT_SECONDARY_LOC, - line: 4, - endLine: 4, - column: 17, - endColumn: 23, - }, - ], - }, - { - code: ` // with default import aliases - import express from 'express'; - import cors from 'cors'; - app.use(cors());`, - errors: [ - { - message: EXPECTED_MESSAGE_WITHOUT_SECONDARY_LOC, - line: 4, - endLine: 4, - column: 17, - endColumn: 23, - }, - ], - }, - { - code: ` - // renaming imported module as something else shouldn't matter. - import * as express from 'express'; - import * as c from 'cors'; - app.use(c());`, - errors: [ - { - message: EXPECTED_MESSAGE_WITHOUT_SECONDARY_LOC, - line: 5, - endLine: 5, - column: 17, - endColumn: 20, - }, - ], - }, - { - code: ` - const express = require('express'); - const cors = require('cors'); - app.use(cors());`, - errors: 1, - }, - { - code: ` - res.setHeader('Access-Control-Allow-Origin', '*' ); // Sensitive - `, - errors: [ - { - message: EXPECTED_MESSAGE_WITHOUT_SECONDARY_LOC, - line: 2, - endLine: 2, - column: 9, - endColumn: 59, - }, - ], - }, - { - code: ` - res.header('Access-Control-Allow-Origin', '*' ); // Sensitive - `, - errors: 1, - }, - { - code: ` - res.set('Access-Control-Allow-Origin', '*' ); // Sensitive - `, - errors: 1, - }, - { - code: ` - res.set({ - 'Content-Type': 'text/plain', - 'Content-Length': '123', - 'Access-Control-Allow-Origin': '*' - }); - const headers = { - 'Content-Type': 'text/plain', - 'Content-Length': '123', - 'Access-Control-Allow-Origin': 'http://localhost', - 'Access-Control-Allow-Origin': '*' - }; - `, - errors: [ - { - message: EXPECTED_MESSAGE_WITHOUT_SECONDARY_LOC, - line: 5, - endLine: 5, - column: 11, - endColumn: 45, - }, - { - message: EXPECTED_MESSAGE_WITHOUT_SECONDARY_LOC, - line: 11, - endLine: 11, - column: 11, - endColumn: 45, - }, - ], - }, - { - code: ` - const express = require('express'); - const cors = require('cors'); - app.use(cors({ - origin: '*', // Sensitive - optionsSuccessStatus: 200 - })); - `, - errors: [ - { - message: EXPECTED_MESSAGE_WITHOUT_SECONDARY_LOC, - line: 5, - endLine: 5, - column: 9, - endColumn: 20, - }, - ], - }, - { - code: ` - const express = require('express'); - const cors = require('cors'); - let corsOptions = { - origin: '*', // Sensitive - optionsSuccessStatus: 200 - }; - app.use(cors(corsOptions)); - `, - errors: [ - { - message: JSON.stringify({ - message: EXPECTED_MESSAGE, - secondaryLocations: [{ column: 19, line: 8, endColumn: 30, endLine: 8 }], - }), - line: 5, - endLine: 5, - column: 9, - endColumn: 20, - }, - ], - }, - { - code: ` - const express = require('express'); - const cors = require('cors'); - function foo() { - let corsOptions = { - origin: 'http://localhost:1234', - optionsSuccessStatus: 200 - }; - app.use(cors(corsOptions)); - } - - function bar() { - let corsOptions = { - origin: '*', // Sensitive - optionsSuccessStatus: 200 - }; - app.use(cors(corsOptions)); - } - `, - errors: [ - { - message: JSON.stringify({ - message: EXPECTED_MESSAGE, - secondaryLocations: [{ column: 21, line: 17, endColumn: 32, endLine: 17 }], - }), - line: 14, - endLine: 14, - column: 11, - endColumn: 22, - }, - ], - }, - { - code: ` - // inlined require('cors') invocation - const express = require('express'); - app.use(require('cors')({ origin: '*' })); // Sensitive - `, - errors: 1, - }, - ], -}); diff --git a/sonar-plugin/javascript-checks/src/main/resources/org/sonar/l10n/javascript/rules/javascript/S5122.html b/sonar-plugin/javascript-checks/src/main/resources/org/sonar/l10n/javascript/rules/javascript/S5122.html index 5fce79a2163..cb2c17e0a95 100644 --- a/sonar-plugin/javascript-checks/src/main/resources/org/sonar/l10n/javascript/rules/javascript/S5122.html +++ b/sonar-plugin/javascript-checks/src/main/resources/org/sonar/l10n/javascript/rules/javascript/S5122.html @@ -49,7 +49,7 @@

Sensitive Code Example

User-controlled origin:

 function (req, res) {
-  const origin = req.header('Origin');
+  const origin = req.headers.origin;
   res.setHeader('Access-Control-Allow-Origin', origin); // Sensitive
 };
@@ -77,9 +77,9 @@

Compliant Solution

User-controlled origin validated with an allow-list:

 function (req, res) {
-  const origin = req.header('Origin');
+  const origin = req.headers.origin;
 
-  if (trustedOrigins.indexOf(origin) >= 0) {
+  if (origin === 'trustedwebsite.com') {
     res.setHeader('Access-Control-Allow-Origin', origin);
   }
 };