Skip to content

Commit 3fb64ab

Browse files
fix consistency and spelling in the documentation
suggestions from the documentation team Co-Authored-By: shati-patel <42641846+shati-patel@users.noreply.github.com>
1 parent c4f27ed commit 3fb64ab

File tree

10 files changed

+24
-24
lines changed

10 files changed

+24
-24
lines changed

change-notes/1.23/analysis-javascript.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
| **Query** | **Tags** | **Purpose** |
1515
|---------------------------------------------------------------------------|-------------------------------------------------------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
1616
| 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. |
17-
| 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. |
17+
| 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 indefinitely. Results are not shown on LGTM by default. |
1818

1919
## Changes to existing queries
2020

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@
3636
This is not secure since an attacker can control the value of
3737
<code>obj.length</code>, and thereby cause the loop to iterate
3838
indefinitely. Here the potential DoS is fixed by enforcing that
39-
the user controlled object is an array.
39+
the user-controlled object is an array.
4040
</p>
4141

4242
<sample src="examples/LoopBoundInjection_fixed.js" />

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ app.post("/foo", (req, res) => {
66

77
var ret = [];
88

9-
// potential DoS if obj.length is large.
9+
// Potential DoS if obj.length is large.
1010
for (var i = 0; i < obj.length; i++) {
1111
ret.push(obj[i]);
1212
}
13-
});
13+
});

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ var app = express();
44
app.post("/foo", (req, res) => {
55
var obj = req.body;
66

7-
if (!(obj instanceof Array)) { // prevents DoS
7+
if (!(obj instanceof Array)) { // Prevents DoS.
88
return [];
99
}
1010

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/**
22
* Provides a taint tracking configuration for reasoning about DoS attacks
3-
* using a user controlled object with an unbounded .length property.
3+
* using a user-controlled object with an unbounded .length property.
44
*
55
* Note, for performance reasons: only import this file if
66
* `LoopBoundInjection::Configuration` is needed, otherwise
@@ -14,7 +14,7 @@ module LoopBoundInjection {
1414
import LoopBoundInjectionCustomizations::LoopBoundInjection
1515

1616
/**
17-
* A taint-tracking configuration for reasoning about looping on tainted objects with unbounded length.
17+
* A taint tracking configuration for reasoning about looping on tainted objects with unbounded length.
1818
*/
1919
class Configuration extends TaintTracking::Configuration {
2020
Configuration() { this = "LoopBoundInjection" }

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/**
2-
* Provides default sources, sinks and sanitisers for reasoning about
2+
* Provides default sources, sinks, and sanitizers for reasoning about
33
* DoS attacks using objects with unbounded length property,
44
* as well as extension points for adding your own.
55
*/
@@ -56,7 +56,7 @@ module LoopBoundInjection {
5656
}
5757

5858
/**
59-
* Gets the length read in the loop test
59+
* Gets the length read in the loop test.
6060
*/
6161
DataFlow::PropRead getLengthRead() { result = lengthRead }
6262

@@ -200,7 +200,7 @@ module LoopBoundInjection {
200200
isCrashingWithNullValues(throws)
201201
)
202202
or
203-
// similar to the loop sink - the existence of an early-exit usually means that no DoS can happen.
203+
// Similar to the loop sink - the existence of an early-exit usually means that no DoS can happen.
204204
exists(ThrowStmt throw |
205205
throw.getTarget() = func.asExpr()
206206
)
@@ -259,7 +259,7 @@ module LoopBoundInjection {
259259
/**
260260
* A sanitizer that blocks taint flow if the length of an array is limited.
261261
*
262-
* Also implicitly makes sure that only the first DoS-prone loop is selected by the query. (as the .length test has outcome=false when exiting the loop).
262+
* Also implicitly makes sure that only the first DoS-prone loop is selected by the query (as the .length test has outcome=false when exiting the loop).
263263
*/
264264
class LengthCheckSanitizerGuard extends TaintTracking::LabeledSanitizerGuardNode,
265265
DataFlow::ValueNode {

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,15 +42,15 @@ function useLengthIndirectly(val) {
4242
}
4343
}
4444

45-
// the obvious null-pointer detection should not hit this one.
45+
// The obvious null-pointer detection should not hit this one.
4646
function noNullPointer(val) {
4747
var ret = [];
4848

4949
const c = 0;
5050

5151
for (var i = 0; i < val.length; i++) { // NOT OK!
5252

53-
// constantly accessing element 0, therefore not guaranteed null-pointer.
53+
// Constantly accessing element 0, therefore not guaranteed null-pointer.
5454
ret.push(val[c].foo);
5555
}
56-
}
56+
}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ function throws(val) {
3636
try {
3737
throw 2; // Is caught, and therefore the DoS is not prevented.
3838
} catch(e) {
39-
// ignored
39+
// Ignored.
4040
}
4141
}
4242
ret.push(val[i]);
@@ -60,9 +60,9 @@ function lodashThrow(val) { // NOT OK!
6060
_.map(val, function (e) {
6161
if (!e) {
6262
try {
63-
throw new Error(); // Does not prevent DoS
63+
throw new Error(); // Does not prevent DoS.
6464
} catch(e) {
65-
// ignored.
65+
// Ignored.
6666
}
6767
}
6868
})

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ function breaks(val) {
1919

2020
for (var i = 0; i < val.length; i++) { // OK
2121
if (val[i] == null) {
22-
break; // prevents DoS.
22+
break; // Prevents DoS.
2323
}
2424
ret.push(val[i]);
2525
}
@@ -30,7 +30,7 @@ function throws(val) {
3030

3131
for (var i = 0; i < val.length; i++) { // OK
3232
if (val[i] == null) {
33-
throw 2; // prevents DoS.
33+
throw 2; // Prevents DoS.
3434
}
3535
ret.push(val[i]);
3636
}
@@ -42,7 +42,7 @@ function returns(val) {
4242

4343
for (var i = 0; i < val.length; i++) { // OK
4444
if (val[i] == null) {
45-
return 2; // prevents DoS.
45+
return 2; // Prevents DoS.
4646
}
4747
ret.push(val[i]);
4848
}
@@ -51,7 +51,7 @@ function returns(val) {
5151
function lodashThrow(val) {
5252
_.map(val, function (e) { // OK
5353
if (!e) {
54-
throw new Error(); // prevents DoS.
54+
throw new Error(); // Prevents DoS.
5555
}
5656
})
5757
}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ function sanitized(val) {
2020
if (!Array.isArray(val)) {
2121
return [];
2222
}
23-
// At this point we know that val must be an Array, and an attacked is
23+
// At this point we know that val must be an Array, and an attacker is
2424
// therefore not able to send a cheap request that spends a lot of time
2525
// inside the loop.
2626
for (var i = 0; i < val.length; i++) { // OK
@@ -50,7 +50,7 @@ function sanitized3(val) {
5050
if (!isArray(val)) {
5151
return [];
5252
}
53-
// At this point we know that val must be an Array, and an attacked is
53+
// At this point we know that val must be an Array, and an attacker is
5454
// therefore not able to send a cheap request that spends a lot of time
5555
// inside the loop.
5656
for (var i = 0; i < val.length; i++) { // OK
@@ -64,7 +64,7 @@ function sanitized4(val) {
6464
if (!(val instanceof Array)) {
6565
return [];
6666
}
67-
// At this point we know that val must be an Array, and an attacked is
67+
// At this point we know that val must be an Array, and an attacker is
6868
// therefore not able to send a cheap request that spends a lot of time
6969
// inside the loop.
7070
for (var i = 0; i < val.length; i++) { // OK

0 commit comments

Comments
 (0)