change DOS to DoS, and other small documentation fixes

Co-Authored-By: Max Schaefer <max@semmle.com>

Author: erik-krogh

change-notes/1.23/analysis-javascript.md

@@ -13,8 +13,8 @@
 
 | **Query**                                                                 | **Tags**                                                          | **Purpose**                                                                                                                                                                            |
 |---------------------------------------------------------------------------|-------------------------------------------------------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
-| Unused index variable (`js/unused-index-variable`)                        | correctness                                                       | Highlights loops that iterate over an array, but do not use the index variable to access array elements, indicating a possible typo or logic error. |
-| Tainted .length in loop condition (`js/loop-bound-injection`)             | security                                                          | Highlights loops where a user-controlled object with an arbitrary .length value can trick the server to loop infinitely. |
+| Unused index variable (`js/unused-index-variable`)                        | correctness                                                       | Highlights loops that iterate over an array, but do not use the index variable to access array elements, indicating a possible typo or logic error. Results are shown on LGTM by default. |
+| Tainted .length in loop condition (`js/loop-bound-injection`)             | security                                                          | Highlights loops where a user-controlled object with an arbitrary .length value can trick the server to loop infinitely. Results are not shown on LGTM by default. |
 
 ## Changes to existing queries
 

javascript/ql/src/Security/CWE-834/TaintedLength.qhelp

@@ -5,7 +5,7 @@
 
 <overview>
 	<p>
-		Using the .length property of an untrusted object as a loop bound may
+		Using the <code>.length</code> property of an untrusted object as a loop bound may
 		cause indefinite looping since a malicious attacker can set the
 		<code>.length</code> property to a very large number. For example, 
 		when a program that expects an array is passed a JSON object such as

javascript/ql/src/Security/CWE-834/TaintedLength.ql

@@ -1,7 +1,7 @@
 /**
  * @name Tainted .length in loop condition
  * @description Iterating over an object with a user-controlled .length
- *              property can cause indefinite looping
+ *              property can cause indefinite looping.
  * @kind path-problem
  * @problem.severity warning
  * @id js/loop-bound-injection

javascript/ql/src/Security/CWE-834/examples/TaintedLength_fixed.js

@@ -4,7 +4,7 @@ var app = express();
 app.post("/foo", (req, res) => {
     var obj = req.body;
 
-    if (!(obj instanceof Array)) { // prevents DOS
+    if (!(obj instanceof Array)) { // prevents DoS
         return [];
     }
 
@@ -13,4 +13,4 @@ app.post("/foo", (req, res) => {
     for (var i = 0; i < obj.length; i++) {
         ret.push(obj[i]);
     }
-});
+});

javascript/ql/src/semmle/javascript/security/dataflow/TaintedLengthCustomizations.qll

@@ -96,7 +96,7 @@ module TaintedLength {
           arrayRead.flowsToExpr(throws) and
           isCrashingWithNullValues(throws)
         ) and
-        // The existence of some kind of early-exit usually indicates that the loop will stop early and no DOS happens.
+        // The existence of some kind of early-exit usually indicates that the loop will stop early and no DoS happens.
         not exists(BreakStmt br | br.getTarget() = loop) and
         not exists(ReturnStmt ret |
           ret.getParentStmt*() = loop.getBody() and
@@ -111,7 +111,7 @@ module TaintedLength {
   }
 
   /**
-   * Holds if `name` is a method from lodash vulnerable to a DOS attack if called with a tained object.
+   * Holds if `name` is a method from lodash vulnerable to a DoS attack if called with a tainted object.
    */
   predicate loopableLodashMethod(string name) {
     name = "chunk" or
@@ -200,7 +200,7 @@ module TaintedLength {
               isCrashingWithNullValues(throws)
             )
             or
-            // similar to the loop sink - the existence of an early-exit usually means that no DOS can happen.
+            // similar to the loop sink - the existence of an early-exit usually means that no DoS can happen.
             exists(ThrowStmt throw |
               throw.getTarget() = func.asExpr()
             )

javascript/ql/test/query-tests/Security/CWE-834/TaintedLengthExitBad.js

@@ -20,7 +20,7 @@ function breaks(val) {
 	for (var i = 0; i < val.length; i++) { // NOT OK!
 	  for (var k = 0; k < 2; k++) {
 		  if (k == 3) {
-		      // Does not prevent DOS, because this is inside an inner loop.
+		      // Does not prevent DoS, because this is inside an inner loop.
 			  break; 
 		  }
 	  }
@@ -34,7 +34,7 @@ function throws(val) {
 	for (var i = 0; i < val.length; i++) { // NOT OK!
 	  if (val[i] == null) {
 		  try {
-			  throw 2; // Is catched, and therefore the DOS is not prevented.
+			  throw 2; // Is caught, and therefore the DoS is not prevented.
 		  } catch(e) {
 			  // ignored
 		  }
@@ -49,7 +49,7 @@ function returns(val) {
 	for (var i = 0; i < val.length; i++) { // NOT OK!
 	  if (val[i] == null) {
 		  (function (i) {
-			  return i+2; // Does not prevent DOS.
+			  return i+2; // Does not prevent DoS.
 		  })(i);
 	  }
       ret.push(val[i]);
@@ -60,10 +60,10 @@ function lodashThrow(val) { // NOT OK!
 	_.map(val, function (e) {
 		if (!e) {
 			try {
-				throw new Error(); // Does not prevent DOS
+				throw new Error(); // Does not prevent DoS
 			} catch(e) {
 				// ignored.
 			}
 		}
 	})
-}
+}

javascript/ql/test/query-tests/Security/CWE-834/TaintedLengthExitGood.js

@@ -19,7 +19,7 @@ function breaks(val) {
 
 	for (var i = 0; i < val.length; i++) { // OK
 	  if (val[i] == null) {
-		  break; // prevents DOS.
+		  break; // prevents DoS.
 	  }
       ret.push(val[i]);
     }
@@ -30,7 +30,7 @@ function throws(val) {
 
 	for (var i = 0; i < val.length; i++) { // OK
 	  if (val[i] == null) {
-		  throw 2; // prevents DOS.
+		  throw 2; // prevents DoS.
 	  }
       ret.push(val[i]);
     }
@@ -42,7 +42,7 @@ function returns(val) {
 
 	for (var i = 0; i < val.length; i++) { // OK
 	  if (val[i] == null) {
-		  return 2; // prevents DOS.
+		  return 2; // prevents DoS.
 	  }
       ret.push(val[i]);
     }
@@ -51,7 +51,7 @@ function returns(val) {
 function lodashThrow(val) {
 	_.map(val, function (e) { // OK
 		if (!e) {
-			throw new Error(); // prevents DOS.
+			throw new Error(); // prevents DoS.
 		}
 	})
-}
+}

javascript/ql/test/query-tests/Security/CWE-834/TaintedLengthObviousLengthCheck.js

@@ -11,12 +11,12 @@ rootRoute.post(function(req, res) {
 function problem(val) {
     var ret = [];
 
-    // Prevents DOS
+    // Prevents DoS
     if (val.length > 100) {
         return [];
     }
 
     for (var i = 0; i < val.length; i++) { // OK
         ret.push(val[i]);
     }
-}
+}