Skip to content

Commit 087c555

Browse files
asgerfasgerf
authored
Merge pull request #18670 from asgerf/js/test-suite
JS: Update test suite to use post-processed inline expectations
2 parents a4f2264 + 6499e54 commit 087c555

File tree

1,106 files changed

+12915
-13112
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

1,106 files changed

+12915
-13112
lines changed

Diff for: ‎javascript/ql/lib/semmle/javascript/frameworks/SQL.qll

+4-1
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,10 @@ private module Postgres {
221221

222222
/** Gets a value that is plugged into a raw placeholder variable, making it a sink for SQL injection. */
223223
private DataFlow::Node getARawValue() {
224-
result = this.getValues() and this.getARawParameterName() = "1" // Special case: if the argument is not an array or object, it's just plugged into $1
224+
result = this.getValues() and
225+
this.getARawParameterName() = "1" and // Special case: if the argument is not an array or object, it's just plugged into $1
226+
not result instanceof DataFlow::ArrayCreationNode and
227+
not result instanceof DataFlow::ObjectLiteralNode
225228
or
226229
exists(DataFlow::SourceNode values | values = this.getValues().getALocalSource() |
227230
result = values.getAPropertyWrite(this.getARawParameterName()).getRhs()

Diff for: ‎javascript/ql/lib/semmle/javascript/frameworks/UriLibraries.qll

+19
Original file line numberDiff line numberDiff line change
@@ -421,3 +421,22 @@ private module ClosureLibraryUri {
421421
}
422422
}
423423
}
424+
425+
private class QueryStringStringification extends DataFlow::SummarizedCallable {
426+
QueryStringStringification() { this = "query-string stringification" }
427+
428+
override DataFlow::InvokeNode getACall() {
429+
result =
430+
API::moduleImport(["querystring", "query-string", "querystringify", "qs"])
431+
.getMember("stringify")
432+
.getACall() or
433+
result = API::moduleImport("url-parse").getMember("qs").getMember("stringify").getACall() or
434+
result = API::moduleImport("parseqs").getMember("encode").getACall()
435+
}
436+
437+
override predicate propagatesFlow(string input, string output, boolean preservesValue) {
438+
preservesValue = false and
439+
input = ["Argument[0]", "Argument[0].AnyMemberDeep"] and
440+
output = "ReturnValue"
441+
}
442+
}

Diff for: ‎javascript/ql/lib/semmle/javascript/security/dataflow/ServerSideUrlRedirectQuery.qll

+8-2
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,11 @@ module ServerSideUrlRedirectConfig implements DataFlow::ConfigSig {
2020

2121
predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
2222

23-
predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
23+
predicate isBarrier(DataFlow::Node node) {
24+
node instanceof Sanitizer
25+
or
26+
node = HostnameSanitizerGuard::getABarrierNode()
27+
}
2428

2529
predicate isBarrierOut(DataFlow::Node node) { hostnameSanitizingPrefixEdge(node, _) }
2630

@@ -69,10 +73,12 @@ deprecated class Configuration extends TaintTracking::Configuration {
6973
}
7074

7175
/**
76+
* DEPRECATED. This is no longer used as a sanitizer guard.
77+
*
7278
* A call to a function called `isLocalUrl` or similar, which is
7379
* considered to sanitize a variable for purposes of URL redirection.
7480
*/
75-
class LocalUrlSanitizingGuard extends DataFlow::CallNode {
81+
deprecated class LocalUrlSanitizingGuard extends DataFlow::CallNode {
7682
LocalUrlSanitizingGuard() { this.getCalleeName().regexpMatch("(?i)(is_?)?local_?url") }
7783

7884
/** DEPRECATED. Use `blocksExpr` instead. */

Diff for: ‎javascript/ql/lib/utils/test/internal/InlineExpectationsTestImpl.qll

+12-4
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,22 @@ private import codeql.util.test.InlineExpectationsTest
44
module Impl implements InlineExpectationsTestSig {
55
private import javascript
66

7-
final private class LineCommentFinal = LineComment;
7+
final class ExpectationComment = ExpectationCommentImpl;
88

9-
class ExpectationComment extends LineCommentFinal {
10-
string getContents() { result = this.getText() }
9+
class Location = JS::Location;
10+
11+
abstract private class ExpectationCommentImpl extends Locatable {
12+
abstract string getContents();
1113

1214
/** Gets this element's location. */
1315
Location getLocation() { result = super.getLocation() }
1416
}
1517

16-
class Location = JS::Location;
18+
private class JSComment extends ExpectationCommentImpl instanceof Comment {
19+
override string getContents() { result = super.getText() }
20+
}
21+
22+
private class HtmlComment extends ExpectationCommentImpl instanceof HTML::CommentNode {
23+
override string getContents() { result = super.getText() }
24+
}
1725
}

Diff for: ‎javascript/ql/src/change-notes/2025-02-21-test-suite.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
category: fix
3+
---
4+
* Fixed a recently-introduced bug that caused `js/server-side-unvalidated-url-redirection` to ignore
5+
valid hostname checks and report spurious alerts after such a check. The original behaviour has been restored.

Diff for: ‎javascript/ql/test/query-tests/AngularJS/DeadAngularJSEventListener/DeadAngularJSEventListener.qlref

+2-1
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
1-
AngularJS/DeadAngularJSEventListener.ql
1+
query: AngularJS/DeadAngularJSEventListener.ql
2+
postprocess: utils/test/InlineExpectationsTestQuery.ql

Diff for: ‎javascript/ql/test/query-tests/AngularJS/DeadAngularJSEventListener/tst.js

+13-13
Original file line numberDiff line numberDiff line change
@@ -1,50 +1,50 @@
11
angular.module('myModule', [])
22
.controller('MyController', function($scope) {
3-
$scope.$on('destroy', cleanup); // BAD
3+
$scope.$on('destroy', cleanup); // $ Alert
44
})
55
.controller('MyController', ["$scope", function(s) {
6-
s.$on('destroy', cleanup); // BAD
6+
s.$on('destroy', cleanup); // $ Alert
77
}])
88
.controller('MyController', function($scope) {
99
var destroy = 'destroy';
10-
$scope.$on(destroy, cleanup); // BAD
10+
$scope.$on(destroy, cleanup); // $ Alert
1111
})
1212
.controller('MyController', function($scope) {
13-
$scope.$on('$destroy', cleanup); // GOOD
13+
$scope.$on('$destroy', cleanup);
1414
})
1515
.controller('MyController', function($scope) {
1616
$scope.$emit('foo');
17-
$scope.$on('foo', cleanup); // GOOD
17+
$scope.$on('foo', cleanup);
1818
})
1919
.controller('MyController', function($scope) {
20-
$scope.$on('bar', cleanup); // BAD
20+
$scope.$on('bar', cleanup); // $ Alert
2121
})
2222
.controller('MyController', function($scope) {
23-
$scope.$on('$locationChangeStart', cleanup); // OK
23+
$scope.$on('$locationChangeStart', cleanup);
2424
})
2525
.controller('MyController', function($scope) {
26-
$scope.$on('lib1.foo', cleanup); // OK
26+
$scope.$on('lib1.foo', cleanup);
2727
})
2828
.controller('MyController', function($scope) {
29-
$scope.$on('lib2:foo', cleanup); // OK
29+
$scope.$on('lib2:foo', cleanup);
3030
})
3131
.controller('MyController', function($scope) {
32-
$scope.$on('onClick', cleanup); // OK
32+
$scope.$on('onClick', cleanup);
3333
})
3434
.controller('MyController', function($scope) {
3535
function f($scope){
3636
$scope.$emit('probablyFromUserCode1')
3737
}
38-
$scope.$on('probablyFromUserCode1', cleanup); // OK
38+
$scope.$on('probablyFromUserCode1', cleanup);
3939
})
4040
.controller('MyController', function($scope) {
4141
function f($scope){
4242
var scope = $scope;
4343
scope.$emit('probablyFromUserCode2')
4444
}
45-
$scope.$on('probablyFromUserCode2', cleanup); // OK
45+
$scope.$on('probablyFromUserCode2', cleanup);
4646
})
4747
.controller('MyController', function($scope) {
48-
$scope.$on('event-from-AngularJS-expression', cleanup); // GOOD
48+
$scope.$on('event-from-AngularJS-expression', cleanup);
4949
})
5050
;

Diff for: ‎javascript/ql/test/query-tests/AngularJS/DependencyMismatch/DependencyMismatch.qlref

+2-1
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
1-
AngularJS/DependencyMismatch.ql
1+
query: AngularJS/DependencyMismatch.ql
2+
postprocess: utils/test/InlineExpectationsTestQuery.ql

Diff for: ‎javascript/ql/test/query-tests/AngularJS/DependencyMismatch/tst.js

+8-8
Original file line numberDiff line numberDiff line change
@@ -1,36 +1,36 @@
11
angular.module('app1', [])
2-
.run(['dep1', 'dep2', 'dep3', function(dep1, dep3, dep2) {}]); // NOT OK
2+
.run(['dep1', 'dep2', 'dep3', function(dep1, dep3, dep2) {}]); // $ Alert
33

44
angular.module('app2')
5-
.directive('mydirective', [ '$compile', function($compile, $http) { // NOT OK
5+
.directive('mydirective', [ '$compile', function($compile, $http) { // $ Alert
66
// ...
77
}]);
88

99
angular.module('app1', [])
10-
.run(['dep1', 'dep2', 'dep3', function(dep1, dep2, dep3) {}]); // OK
10+
.run(['dep1', 'dep2', 'dep3', function(dep1, dep2, dep3) {}]);
1111

1212
angular.module('app2')
13-
.directive('mydirective', [ '$compile', '$http', function($compile, $http) { // OK
13+
.directive('mydirective', [ '$compile', '$http', function($compile, $http) {
1414
// ...
1515
}]);
1616

1717
angular.module('app3', [])
18-
.run(function(dep1, dep3) {}); // OK
18+
.run(function(dep1, dep3) {});
1919

2020
angular.module('app4')
21-
.directive('mydirective', function($compile, $http) { // OK
21+
.directive('mydirective', function($compile, $http) {
2222
// ...
2323
});
2424

2525
angular.module('app5')
26-
.directive('mydirective', [ 'fully.qualified.name', function(name) { // OK
26+
.directive('mydirective', [ 'fully.qualified.name', function(name) {
2727
// ...
2828
}])
2929

3030
angular.module('app6')
3131
.directive('mydirective', function() {
3232
return {
33-
link: function (scope, element, attrs) { // OK
33+
link: function (scope, element, attrs) {
3434
}
3535
};
3636
});

Diff for: ‎javascript/ql/test/query-tests/AngularJS/DisablingSce/DisablingSce.js

+5-5
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,17 @@
11
angular.module('app', [])
22
.config(function($sceProvider) {
3-
$sceProvider.enabled(false); // BAD
3+
$sceProvider.enabled(false); // $ Alert
44
})
55
.config(['otherProvider', function($sceProvider) {
6-
$sceProvider.enabled(false); // OK
6+
$sceProvider.enabled(false);
77
}])
88
.config(['$sceProvider', function(x) {
9-
x.enabled(false); // BAD
9+
x.enabled(false); // $ Alert
1010
}])
1111
.config(function($sceProvider) {
12-
$sceProvider.enabled(true); // OK
12+
$sceProvider.enabled(true);
1313
})
1414
.config(function($sceProvider) {
1515
var x = false;
16-
$sceProvider.enabled(x); // BAD
16+
$sceProvider.enabled(x); // $ Alert
1717
});

Diff for: ‎javascript/ql/test/query-tests/AngularJS/DisablingSce/DisablingSce.qlref

+2-1
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
1-
AngularJS/DisablingSce.ql
1+
query: AngularJS/DisablingSce.ql
2+
postprocess: utils/test/InlineExpectationsTestQuery.ql

Diff for: ‎javascript/ql/test/query-tests/AngularJS/DoubleCompilation/DoubleCompilation.qlref

+2-1
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
1-
AngularJS/DoubleCompilation.ql
1+
query: AngularJS/DoubleCompilation.ql
2+
postprocess: utils/test/InlineExpectationsTestQuery.ql

Diff for: ‎javascript/ql/test/query-tests/AngularJS/DoubleCompilation/bad.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ angular.module('app').directive('addMouseover', function($compile) {
1111

1212
attrs.$set('addMouseover', null); // To stop infinite compile loop
1313
element.append(newEl);
14-
$compile(element)(scope); // Double compilation
14+
$compile(element)(scope); // $ Alert - Double compilation
1515
}
1616
}
1717
})

Diff for: ‎javascript/ql/test/query-tests/AngularJS/IncompatibleService/IncompatibleService.qlref

+2-1
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
1-
AngularJS/IncompatibleService.ql
1+
query: AngularJS/IncompatibleService.ql
2+
postprocess: utils/test/InlineExpectationsTestQuery.ql

Diff for: ‎javascript/ql/test/query-tests/AngularJS/IncompatibleService/angular-incompatible-service.js

+58-58
Original file line numberDiff line numberDiff line change
@@ -11,68 +11,68 @@ angular.module('myModule', [])
1111
;
1212

1313
angular.module('myModule2', [])
14-
.controller('c0', function(factoryId){}) // OK
15-
.controller('c1', function(serviceId){}) // OK
16-
.controller('c2', function(valueId){}) // OK
17-
.controller('c3', function(constantId){}) // OK
18-
.controller('c4', function(providerId){}) // OK
19-
.controller('c5', function($http){}) // OK
20-
.controller('c6', function($provider){}) // NOT OK
21-
.controller('c7', function($scope){}) // OK
22-
.controller('c8', function($compile){}) // OK
23-
.controller('c9', function(UNKNOWN){}) // OK
24-
.controller('c10', function(providerIdProvider){}) // NOT OK
25-
.controller('c11', function(providerIdProvider, UNKNOWN){}) // NOT OK, but only one error
26-
.controller('c12', function($provide){}) // OK (special case)
27-
.controller('c13', function(providerId2Provider){}) // NOT OK
14+
.controller('c0', function(factoryId){})
15+
.controller('c1', function(serviceId){})
16+
.controller('c2', function(valueId){})
17+
.controller('c3', function(constantId){})
18+
.controller('c4', function(providerId){})
19+
.controller('c5', function($http){})
20+
.controller('c6', function($provider){}) // $ Alert
21+
.controller('c7', function($scope){})
22+
.controller('c8', function($compile){})
23+
.controller('c9', function(UNKNOWN){})
24+
.controller('c10', function(providerIdProvider){}) // $ Alert
25+
.controller('c11', function(providerIdProvider, UNKNOWN){}) // $ Alert - but only one error
26+
.controller('c12', function($provide){}) // OK - special case
27+
.controller('c13', function(providerId2Provider){}) // $ Alert
2828

29-
.factory('s0', function(factoryId){}) // OK
30-
.factory('s1', function(serviceId){}) // OK
31-
.factory('s2', function(valueId){}) // OK
32-
.factory('s3', function(constantId){}) // OK
33-
.factory('s4', function(providerId){}) // OK
34-
.factory('s5', function($http){}) // OK
35-
.factory('s6', function($provider){}) // NOT OK
36-
.factory('s7', function($scope){}) // NOT OK
37-
.factory('s8', function($compile){}) // OK
38-
.factory('s9', function(UNKNOWN){}) // OK
39-
.factory('s10', function(providerIdProvider){}) // NOT OK
40-
.factory('s11', function(providerIdProvider, UNKNOWN){}) // NOT OK, but only one error
41-
.factory('s12', function($provide){}) // OK (special case)
42-
.factory('s13', function(providerId2Provider){}) // NOT OK
29+
.factory('s0', function(factoryId){})
30+
.factory('s1', function(serviceId){})
31+
.factory('s2', function(valueId){})
32+
.factory('s3', function(constantId){})
33+
.factory('s4', function(providerId){})
34+
.factory('s5', function($http){})
35+
.factory('s6', function($provider){}) // $ Alert
36+
.factory('s7', function($scope){}) // $ Alert
37+
.factory('s8', function($compile){})
38+
.factory('s9', function(UNKNOWN){})
39+
.factory('s10', function(providerIdProvider){}) // $ Alert
40+
.factory('s11', function(providerIdProvider, UNKNOWN){}) // $ Alert - but only one error
41+
.factory('s12', function($provide){}) // OK - special case
42+
.factory('s13', function(providerId2Provider){}) // $ Alert
4343

44-
.run(function(factoryId){}) // OK
45-
.run(function(serviceId){}) // OK
46-
.run(function(valueId){}) // OK
47-
.run(function(constantId){}) // OK
48-
.run(function(providerId){}) // OK
49-
.run(function($http){}) // OK
50-
.run(function($provider){}) // NOT OK
51-
.run(function($scope){}) // NOT OK
52-
.run(function($compile){}) // OK
53-
.run(function(UNKNOWN){}) // OK
54-
.run(function(providerIdProvider){}) // NOT OK
55-
.run(function(providerIdProvider, UNKNOWN){}) // NOT OK, but only one error
56-
.run(function($provide){}) // OK (special case)
57-
.run(function(providerId2Provider){}) // NOT OK
44+
.run(function(factoryId){})
45+
.run(function(serviceId){})
46+
.run(function(valueId){})
47+
.run(function(constantId){})
48+
.run(function(providerId){})
49+
.run(function($http){})
50+
.run(function($provider){}) // $ Alert
51+
.run(function($scope){}) // $ Alert
52+
.run(function($compile){})
53+
.run(function(UNKNOWN){})
54+
.run(function(providerIdProvider){}) // $ Alert
55+
.run(function(providerIdProvider, UNKNOWN){}) // $ Alert - but only one error
56+
.run(function($provide){}) // OK - special case
57+
.run(function(providerId2Provider){}) // $ Alert
5858

59-
.config(function(factoryId){}) // NOT OK
60-
.config(function(serviceId){}) // NOT OK
61-
.config(function(valueId){}) // NOT OK
62-
.config(function(constantId){}) // OK
63-
.config(function(providerId){}) // NOT OK
64-
.config(function($http){}) // NOT OK
65-
.config(function($provider){}) // OK
66-
.config(function($scope){}) // NOT OK
67-
.config(function($compile){}) // OK
68-
.config(function(UNKNOWN){}) // OK
69-
.config(function(providerIdProvider){}) // OK
70-
.config(function(providerId, UNKNOWN){}) // NOT OK, but only one error
71-
.config(function($provide){}) // OK (special case)
72-
.config(function(valueId2){}) // NOT OK
59+
.config(function(factoryId){}) // $ Alert
60+
.config(function(serviceId){}) // $ Alert
61+
.config(function(valueId){}) // $ Alert
62+
.config(function(constantId){})
63+
.config(function(providerId){}) // $ Alert
64+
.config(function($http){}) // $ Alert
65+
.config(function($provider){})
66+
.config(function($scope){}) // $ Alert
67+
.config(function($compile){})
68+
.config(function(UNKNOWN){})
69+
.config(function(providerIdProvider){})
70+
.config(function(providerId, UNKNOWN){}) // $ Alert - but only one error
71+
.config(function($provide){}) // OK - special case
72+
.config(function(valueId2){}) // $ Alert
7373

7474
// service: same restrcitions as .factory
75-
.service('s14', function(factoryId){}) // OK
76-
.service('s15', function($provider){}) // NOT OK
75+
.service('s14', function(factoryId){})
76+
.service('s15', function($provider){}) // $ Alert
7777

7878
;

Diff for: ‎javascript/ql/test/query-tests/AngularJS/InsecureUrlWhitelist/InsecureUrlWhitelist.qlref

+2-1
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
1-
AngularJS/InsecureUrlWhitelist.ql
1+
query: AngularJS/InsecureUrlWhitelist.ql
2+
postprocess: utils/test/InlineExpectationsTestQuery.ql

0 commit comments

Comments
 (0)