Improve S5689: Use different messages if sensitive header is set expl…

…icitely or by default
SonarSource · Aug 24, 2023 · a4df46b · a4df46b
1 parent 781954d
commit a4df46b
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 12 deletions.
diff --git a/packages/jsts/src/rules/S5689/rule.ts b/packages/jsts/src/rules/S5689/rule.ts
@@ -33,17 +33,20 @@ const APP_SET_NUM_ARGS = 2;
 export const rule: Rule.RuleModule = {
   meta: {
     messages: {
-      disclosingFingerprinting:
-        'Make sure disclosing the fingerprinting of this web technology is safe here.',
+      headerSet: 'Make sure disclosing the fingerprinting of this web technology is safe here.',
+      headerDefault:
+        'This framework implicitly discloses version information by default. Make sure it is safe here.',
     },
   },
   create(context: Rule.RuleContext) {
     let appInstantiation: estree.Identifier | null = null;
     let isSafe = false;
+    let isExplicitelyUnsafe = false;
     return {
       Program() {
         appInstantiation = null;
         isSafe = false;
+        isExplicitelyUnsafe = true;
       },
       CallExpression: (node: estree.Node) => {
         if (!isSafe && appInstantiation) {
@@ -53,6 +56,7 @@ export const rule: Rule.RuleModule = {
             isDisabledXPoweredBy(callExpr, appInstantiation) ||
             isSetFalseXPoweredBy(callExpr, appInstantiation) ||
             isAppEscaping(callExpr, appInstantiation);
+          isExplicitelyUnsafe = isSetTrueXPoweredBy(callExpr, appInstantiation);
         }
       },
       VariableDeclarator: (node: estree.Node) => {
@@ -72,9 +76,13 @@ export const rule: Rule.RuleModule = {
       },
       'Program:exit'() {
         if (!isSafe && appInstantiation) {
+          let messageId = 'headerDefault';
+          if (isExplicitelyUnsafe) {
+            messageId = 'headerSet'
+          }
           context.report({
             node: appInstantiation,
-            messageId: 'disclosingFingerprinting',
+            messageId,
           });
         }
       },
@@ -113,16 +121,28 @@ function isSetFalseXPoweredBy(
   callExpression: estree.CallExpression,
   app: estree.Identifier,
 ): boolean {
+  return getSetTrueXPoweredByValue(callExpression, app) === false;
+}
+
+function isSetTrueXPoweredBy(
+  callExpression: estree.CallExpression,
+  app: estree.Identifier,
+): boolean {
+  return getSetTrueXPoweredByValue(callExpression, app) === true;
+}
+
+function getSetTrueXPoweredByValue(callExpression: estree.CallExpression, app: estree.Identifier) {
   if (isMethodInvocation(callExpression, app.name, 'set', APP_SET_NUM_ARGS)) {
     const [headerName, onOff] = callExpression.arguments;
-    return (
+    if (
       headerName.type === 'Literal' &&
       String(headerName.value).toLowerCase() === HEADER_X_POWERED_BY &&
-      onOff.type === 'Literal' &&
-      onOff.value === false
-    );
+      onOff.type === 'Literal'
+    ) {
+      return onOff.value;
+    }
   }
-  return false;
+  return undefined;
 }
 
 function isAppEscaping(callExpr: estree.CallExpression, app: estree.Identifier): boolean {

diff --git a/packages/jsts/src/rules/S5689/unit.test.ts b/packages/jsts/src/rules/S5689/unit.test.ts
@@ -170,7 +170,7 @@ ruleTester.run(
       `,
         errors: [
           {
-            message: 'Make sure disclosing the fingerprinting of this web technology is safe here.',
+            message: 'This framework implicitly discloses version information by default. Make sure it is safe here.',
             line: 3,
             endLine: 3,
             column: 15,
@@ -185,7 +185,7 @@ ruleTester.run(
       `,
         errors: [
           {
-            message: 'Make sure disclosing the fingerprinting of this web technology is safe here.',
+            message: 'This framework implicitly discloses version information by default. Make sure it is safe here.',
             line: 3,
             endLine: 3,
             column: 15,
@@ -199,7 +199,7 @@ ruleTester.run(
       `,
         errors: [
           {
-            message: 'Make sure disclosing the fingerprinting of this web technology is safe here.',
+            message: 'This framework implicitly discloses version information by default. Make sure it is safe here.',
             line: 2,
             endLine: 2,
             column: 15,
@@ -213,7 +213,15 @@ ruleTester.run(
         const app = express();
         app.set("x-powered-by", true); // Noncompliant
       `,
-        errors: 1,
+        errors: [
+          {
+            message: 'Make sure disclosing the fingerprinting of this web technology is safe here.',
+            line: 3,
+            endLine: 3,
+            column: 15,
+            endColumn: 18,
+          },
+        ],
       },
       {
         code: `