Skip to content

Commit c0c2aa6

Browse files
tausbntausbn
authored
Merge branch 'main' into python-port-flask-to-api-graphs
2 parents 2c4a477 + 8a2e063 commit c0c2aa6

22 files changed

Lines changed: 523 additions & 50 deletions

File tree

‎cpp/ql/src/semmle/code/cpp/commons/Printf.qll‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,12 @@ predicate primitiveVariadicFormatter(
5353
(
5454
if type = "" then outputParamIndex = -1 else outputParamIndex = 0 // Conveniently, these buffer parameters are all at index 0.
5555
) and
56-
not exists(f.getBlock()) // exclude functions with an implementation in the snapshot as they may not be standard implementations.
56+
not (
57+
// exclude functions with an implementation in the snapshot source
58+
// directory, as they may not be standard implementations.
59+
exists(f.getBlock()) and
60+
exists(f.getFile().getRelativePath())
61+
)
5762
}
5863

5964
private predicate callsVariadicFormatter(

‎javascript/extractor/src/com/semmle/js/extractor/HTMLExtractor.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -331,7 +331,10 @@ private void extractSnippet(
331331
textualExtractor.getMetrics(),
332332
textualExtractor.getExtractedFile());
333333
Pair<Label, LoCInfo> result = extractor.extract(tx, source, toplevelKind, scopeManager);
334-
emitTopLevelXmlNodeBinding(parentHtmlNode, result.fst(), context, trapWriter);
334+
Label toplevelLabel = result.fst();
335+
if (toplevelLabel != null) { // can be null when script ends up being parsed as JSON
336+
emitTopLevelXmlNodeBinding(parentHtmlNode, toplevelLabel, context, trapWriter);
337+
}
335338
locInfo.add(result.snd());
336339
} catch (ParseError e) {
337340
e.setPosition(scriptLocationManager.translatePosition(e.getPosition()));

‎javascript/extractor/src/com/semmle/js/extractor/Main.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ public class Main {
4343
* A version identifier that should be updated every time the extractor changes in such a way that
4444
* it may produce different tuples for the same file under the same {@link ExtractorConfig}.
4545
*/
46-
public static final String EXTRACTOR_VERSION = "2020-12-11";
46+
public static final String EXTRACTOR_VERSION = "2021-02-05";
4747

4848
public static final Pattern NEWLINE = Pattern.compile("\n");
4949

‎javascript/extractor/tests/html/input/json_in_script.html‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
<html>
2+
<script>
3+
{"Hello": 123}
4+
</script>
5+
</html>

‎javascript/extractor/tests/html/output/trap/json_in_script.html.trap‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
#10000=@"/json_in_script.html;sourcefile"
2+
files(#10000,"/json_in_script.html","json_in_script","html",0)
3+
#10001=@"/;folder"
4+
folders(#10001,"/","")
5+
containerparent(#10001,#10000)
6+
#10002=@"loc,{#10000},0,0,0,0"
7+
locations_default(#10002,#10000,0,0,0,0)
8+
hasLocation(#10000,#10002)
9+
#20000=@"global_scope"
10+
scopes(#20000,0)
11+
#20001=*
12+
json(#20001,5,#10000,0,"{""Hello"": 123}")
13+
#20002=@"loc,{#10000},3,1,3,14"
14+
locations_default(#20002,#10000,3,1,3,14)
15+
json_locations(#20001,#20002)
16+
#20003=*
17+
json(#20003,2,#20001,0,"123")
18+
#20004=@"loc,{#10000},3,11,3,13"
19+
locations_default(#20004,#10000,3,11,3,13)
20+
json_locations(#20003,#20004)
21+
json_literals("123","123",#20003)
22+
json_properties(#20001,"Hello",#20003)
23+
#20005=*
24+
xmlElements(#20005,"html",#10000,0,#10000)
25+
#20006=@"loc,{#10000},1,1,5,7"
26+
locations_default(#20006,#10000,1,1,5,7)
27+
xmllocations(#20005,#20006)
28+
#20007=*
29+
xmlElements(#20007,"script",#20005,0,#10000)
30+
#20008=@"loc,{#10000},2,1,4,9"
31+
locations_default(#20008,#10000,2,1,4,9)
32+
xmllocations(#20007,#20008)
33+
numlines(#10000,5,0,0)
34+
filetype(#10000,"html")

‎javascript/ql/src/semmle/javascript/dataflow/DataFlow.qll‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1079,8 +1079,19 @@ module DataFlow {
10791079

10801080
override DataFlow::Node getCalleeNode() { result = DataFlow::valueNode(astNode.getCallee()) }
10811081

1082+
/**
1083+
* Whether i is an index that occurs after a spread argument.
1084+
*/
1085+
pragma[nomagic]
1086+
private predicate isIndexAfterSpread(int i) {
1087+
astNode.isSpreadArgument(i)
1088+
or
1089+
exists(astNode.getArgument(i)) and
1090+
isIndexAfterSpread(i - 1)
1091+
}
1092+
10821093
override DataFlow::Node getArgument(int i) {
1083-
not astNode.isSpreadArgument([0 .. i]) and
1094+
not isIndexAfterSpread(i) and
10841095
result = DataFlow::valueNode(astNode.getArgument(i))
10851096
}
10861097

‎python/change-notes/2021-01-19-port-url-redirect-query.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
lgtm,codescanning
2+
* Ported URL redirection (`py/url-redirection`) query to use new data-flow library. This might result in different results, but overall a more robust and accurate analysis.

‎python/ql/src/Security/CWE-601/UrlRedirect.ql‎

Lines changed: 6 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -12,30 +12,10 @@
1212
*/
1313

1414
import python
15-
import semmle.python.security.Paths
16-
import semmle.python.web.HttpRedirect
17-
import semmle.python.web.HttpRequest
18-
import semmle.python.security.strings.Untrusted
15+
import semmle.python.security.dataflow.UrlRedirect
16+
import DataFlow::PathGraph
1917

20-
/** Url redirection is a problem only if the user controls the prefix of the URL */
21-
class UntrustedPrefixStringKind extends UntrustedStringKind {
22-
override TaintKind getTaintForFlowStep(ControlFlowNode fromnode, ControlFlowNode tonode) {
23-
result = UntrustedStringKind.super.getTaintForFlowStep(fromnode, tonode) and
24-
not tonode.(BinaryExprNode).getRight() = fromnode
25-
}
26-
}
27-
28-
class UrlRedirectConfiguration extends TaintTracking::Configuration {
29-
UrlRedirectConfiguration() { this = "URL redirect configuration" }
30-
31-
override predicate isSource(TaintTracking::Source source) {
32-
source instanceof HttpRequestTaintSource
33-
}
34-
35-
override predicate isSink(TaintTracking::Sink sink) { sink instanceof HttpRedirectTaintSink }
36-
}
37-
38-
from UrlRedirectConfiguration config, TaintedPathSource src, TaintedPathSink sink
39-
where config.hasFlowPath(src, sink)
40-
select sink.getSink(), src, sink, "Untrusted URL redirection due to $@.", src.getSource(),
41-
"a user-provided value"
18+
from UrlRedirectConfiguration config, DataFlow::PathNode source, DataFlow::PathNode sink
19+
where config.hasFlowPath(source, sink)
20+
select sink.getNode(), source, sink, "Untrusted URL redirection due to $@.", source.getNode(),
21+
"A user-provided value"

‎python/ql/src/experimental/Security-old-dataflow/CWE-601/UrlRedirect.ql‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
/**
2+
* @name URL redirection from remote source
3+
* @description URL redirection based on unvalidated user input
4+
* may cause redirection to malicious web sites.
5+
* @kind path-problem
6+
* @problem.severity error
7+
* @sub-severity low
8+
* @id py/url-redirection
9+
* @tags security
10+
* external/cwe/cwe-601
11+
* @precision high
12+
*/
13+
14+
import python
15+
import semmle.python.security.Paths
16+
import semmle.python.web.HttpRedirect
17+
import semmle.python.web.HttpRequest
18+
import semmle.python.security.strings.Untrusted
19+
20+
/** Url redirection is a problem only if the user controls the prefix of the URL */
21+
class UntrustedPrefixStringKind extends UntrustedStringKind {
22+
override TaintKind getTaintForFlowStep(ControlFlowNode fromnode, ControlFlowNode tonode) {
23+
result = UntrustedStringKind.super.getTaintForFlowStep(fromnode, tonode) and
24+
not tonode.(BinaryExprNode).getRight() = fromnode
25+
}
26+
}
27+
28+
class UrlRedirectConfiguration extends TaintTracking::Configuration {
29+
UrlRedirectConfiguration() { this = "URL redirect configuration" }
30+
31+
override predicate isSource(TaintTracking::Source source) {
32+
source instanceof HttpRequestTaintSource
33+
}
34+
35+
override predicate isSink(TaintTracking::Sink sink) { sink instanceof HttpRedirectTaintSink }
36+
}
37+
38+
from UrlRedirectConfiguration config, TaintedPathSource src, TaintedPathSink sink
39+
where config.hasFlowPath(src, sink)
40+
select sink.getSink(), src, sink, "Untrusted URL redirection due to $@.", src.getSource(),
41+
"a user-provided value"

‎python/ql/src/semmle/python/Concepts.qll‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -473,5 +473,40 @@ module HTTP {
473473
}
474474
}
475475
}
476+
477+
/**
478+
* A data-flow node that creates a HTTP redirect response on a server.
479+
*
480+
* Note: we don't require that this redirect must be sent to a client (a kind of
481+
* "if a tree falls in a forest and nobody hears it" situation).
482+
*
483+
* Extend this class to refine existing API models. If you want to model new APIs,
484+
* extend `HttpRedirectResponse::Range` instead.
485+
*/
486+
class HttpRedirectResponse extends HttpResponse {
487+
override HttpRedirectResponse::Range range;
488+
489+
HttpRedirectResponse() { this = range }
490+
491+
/** Gets the data-flow node that specifies the location of this HTTP redirect response. */
492+
DataFlow::Node getRedirectLocation() { result = range.getRedirectLocation() }
493+
}
494+
495+
/** Provides a class for modeling new HTTP redirect response APIs. */
496+
module HttpRedirectResponse {
497+
/**
498+
* A data-flow node that creates a HTTP redirect response on a server.
499+
*
500+
* Note: we don't require that this redirect must be sent to a client (a kind of
501+
* "if a tree falls in a forest and nobody hears it" situation).
502+
*
503+
* Extend this class to model new APIs. If you want to refine existing API models,
504+
* extend `HttpResponse` instead.
505+
*/
506+
abstract class Range extends HTTP::Server::HttpResponse::Range {
507+
/** Gets the data-flow node that specifies the location of this HTTP redirect response. */
508+
abstract DataFlow::Node getRedirectLocation();
509+
}
510+
}
476511
}
477512
}

0 commit comments

Comments
 (0)