Skip to content

Commit fbbec5d

Browse files
RasmusWLRasmusWL
authored
Merge pull request #5118 from yoff/python-port-stacktrace-exosure
Python: Port stack trace exposure
2 parents 5097836 + 4a9023b commit fbbec5d

10 files changed

Lines changed: 163 additions & 26 deletions

File tree

‎python/change-notes/2021-02-25-port-stactrace-exposure-query.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
lgtm,codescanning
2+
* Updated _Information exposure through an exception_ (`py/stack-trace-exposure`) query to use the new data-flow library and type-tracking approach instead of points-to analysis. You may see differences in the results found by the query, but overall this change should result in a more robust and accurate analysis.

‎python/ql/src/Security/CWE-209/StackTraceExposure.ql‎

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -13,19 +13,10 @@
1313
*/
1414

1515
import python
16-
import semmle.python.security.Paths
17-
import semmle.python.security.Exceptions
18-
import semmle.python.web.HttpResponse
16+
import semmle.python.security.dataflow.StackTraceExposure
17+
import DataFlow::PathGraph
1918

20-
class StackTraceExposureConfiguration extends TaintTracking::Configuration {
21-
StackTraceExposureConfiguration() { this = "Stack trace exposure configuration" }
22-
23-
override predicate isSource(TaintTracking::Source source) { source instanceof ErrorInfoSource }
24-
25-
override predicate isSink(TaintTracking::Sink sink) { sink instanceof HttpResponseTaintSink }
26-
}
27-
28-
from StackTraceExposureConfiguration config, TaintedPathSource src, TaintedPathSink sink
29-
where config.hasFlowPath(src, sink)
30-
select sink.getSink(), src, sink, "$@ may be exposed to an external user", src.getSource(),
19+
from StackTraceExposureConfiguration config, DataFlow::PathNode source, DataFlow::PathNode sink
20+
where config.hasFlowPath(source, sink)
21+
select sink.getNode(), source, sink, "$@ may be exposed to an external user", source.getNode(),
3122
"Error information"

‎python/ql/src/semmle/python/security/dataflow/ExceptionInfo.qll‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
/** Provides classes representing various sources of information about raised exceptions. */
2+
3+
import python
4+
import semmle.python.dataflow.new.DataFlow
5+
private import semmle.python.ApiGraphs
6+
7+
/**
8+
* INTERNAL: Do not use.
9+
*
10+
* A data-flow node that carries information about a raised exception.
11+
* Such information should rarely be exposed directly to the user.
12+
*/
13+
abstract class ExceptionInfo extends DataFlow::Node { }
14+
15+
/** A call to a function from the `traceback` module revealing information about a raised exception. */
16+
private class TracebackFunctionCall extends ExceptionInfo, DataFlow::CallCfgNode {
17+
TracebackFunctionCall() {
18+
this =
19+
API::moduleImport("traceback")
20+
.getMember([
21+
"extract_tb", "extract_stack", "format_list", "format_exception_only",
22+
"format_exception", "format_exc", "format_tb", "format_stack"
23+
])
24+
.getACall()
25+
}
26+
}
27+
28+
/** A caught exception. */
29+
private class CaughtException extends ExceptionInfo {
30+
CaughtException() {
31+
this.asVar().getDefinition().(EssaNodeDefinition).getDefiningNode().getNode() =
32+
any(ExceptStmt s).getName()
33+
}
34+
}
35+
36+
/** A call to `sys.exc_info`. */
37+
private class SysExcInfoCall extends ExceptionInfo, DataFlow::CallCfgNode {
38+
SysExcInfoCall() { this = API::moduleImport("sys").getMember("exc_info").getACall() }
39+
}

‎python/ql/src/semmle/python/security/dataflow/StackTraceExposure.qll‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
/**
2+
* Provides a taint-tracking configuration for detecting stack trace exposure
3+
* vulnerabilities.
4+
*/
5+
6+
import python
7+
import semmle.python.dataflow.new.DataFlow
8+
import semmle.python.dataflow.new.TaintTracking
9+
import semmle.python.Concepts
10+
import semmle.python.dataflow.new.internal.Attributes
11+
private import ExceptionInfo
12+
13+
/**
14+
* A taint-tracking configuration for detecting stack trace exposure.
15+
*/
16+
class StackTraceExposureConfiguration extends TaintTracking::Configuration {
17+
StackTraceExposureConfiguration() { this = "StackTraceExposureConfiguration" }
18+
19+
override predicate isSource(DataFlow::Node source) { source instanceof ExceptionInfo }
20+
21+
override predicate isSink(DataFlow::Node sink) {
22+
sink = any(HTTP::Server::HttpResponse response).getBody()
23+
}
24+
25+
// A stack trace is accessible as the `__traceback__` attribute of a caught exception.
26+
// seehttps://docs.python.org/3/reference/datamodel.html#traceback-objects
27+
override predicate isAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
28+
exists(AttrRead attr | attr.getAttributeName() = "__traceback__" |
29+
nodeFrom = attr.getObject() and
30+
nodeTo = attr
31+
)
32+
}
33+
}

‎python/ql/test/query-tests/Security/CWE-209/ExceptionInfo.expected‎

Whitespace-only changes.

‎python/ql/test/query-tests/Security/CWE-209/ExceptionInfo.ql‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
import python
2+
import semmle.python.dataflow.new.DataFlow
3+
import TestUtilities.InlineExpectationsTest
4+
import semmle.python.security.dataflow.ExceptionInfo
5+
6+
class ExceptionInfoTest extends InlineExpectationsTest {
7+
ExceptionInfoTest() { this = "ExceptionInfoTest" }
8+
9+
override string getARelevantTag() { result = "exceptionInfo" }
10+
11+
override predicate hasActualResult(Location location, string element, string tag, string value) {
12+
exists(location.getFile().getRelativePath()) and
13+
exists(ExceptionInfo e |
14+
location = e.getLocation() and
15+
element = e.toString() and
16+
value = "" and
17+
tag = "exceptionInfo"
18+
)
19+
}
20+
}

‎python/ql/test/query-tests/Security/CWE-209/Exceptions.py‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
try:
2+
1+2
3+
except Exception as e: #$ exceptionInfo
4+
e
5+
6+
def test_exception():
7+
try:
8+
1+2
9+
except Exception as e: #$ exceptionInfo
10+
e

‎python/ql/test/query-tests/Security/CWE-209/StackTraceExposure.expected‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,19 @@
11
edges
2-
| test.py:33:15:33:36 | exception info | test.py:34:29:34:31 | exception info |
3-
| test.py:33:15:33:36 | exception info | test.py:34:29:34:31 | exception info |
4-
| test.py:34:29:34:31 | exception info | test.py:34:16:34:32 | exception info |
5-
| test.py:34:29:34:31 | exception info | test.py:34:16:34:32 | exception info |
2+
| test.py:23:25:23:25 | SSA variable e | test.py:24:16:24:16 | ControlFlowNode for e |
3+
| test.py:31:25:31:25 | SSA variable e | test.py:32:16:32:30 | ControlFlowNode for Attribute |
4+
| test.py:49:15:49:36 | ControlFlowNode for Attribute() | test.py:50:29:50:31 | ControlFlowNode for err |
5+
| test.py:50:29:50:31 | ControlFlowNode for err | test.py:50:16:50:32 | ControlFlowNode for format_error() |
6+
nodes
7+
| test.py:16:16:16:37 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
8+
| test.py:23:25:23:25 | SSA variable e | semmle.label | SSA variable e |
9+
| test.py:24:16:24:16 | ControlFlowNode for e | semmle.label | ControlFlowNode for e |
10+
| test.py:31:25:31:25 | SSA variable e | semmle.label | SSA variable e |
11+
| test.py:32:16:32:30 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
12+
| test.py:49:15:49:36 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
13+
| test.py:50:16:50:32 | ControlFlowNode for format_error() | semmle.label | ControlFlowNode for format_error() |
14+
| test.py:50:29:50:31 | ControlFlowNode for err | semmle.label | ControlFlowNode for err |
615
#select
7-
| test.py:16:16:16:37 | Attribute() | test.py:16:16:16:37 | exception info | test.py:16:16:16:37 | exception info | $@ may be exposed to an external user | test.py:16:16:16:37 | Attribute() | Error information |
8-
| test.py:34:16:34:32 | format_error() | test.py:33:15:33:36 | exception info | test.py:34:16:34:32 | exception info | $@ may be exposed to an external user | test.py:33:15:33:36 | Attribute() | Error information |
16+
| test.py:16:16:16:37 | ControlFlowNode for Attribute() | test.py:16:16:16:37 | ControlFlowNode for Attribute() | test.py:16:16:16:37 | ControlFlowNode for Attribute() | $@ may be exposed to an external user | test.py:16:16:16:37 | ControlFlowNode for Attribute() | Error information |
17+
| test.py:24:16:24:16 | ControlFlowNode for e | test.py:23:25:23:25 | SSA variable e | test.py:24:16:24:16 | ControlFlowNode for e | $@ may be exposed to an external user | test.py:23:25:23:25 | SSA variable e | Error information |
18+
| test.py:32:16:32:30 | ControlFlowNode for Attribute | test.py:31:25:31:25 | SSA variable e | test.py:32:16:32:30 | ControlFlowNode for Attribute | $@ may be exposed to an external user | test.py:31:25:31:25 | SSA variable e | Error information |
19+
| test.py:50:16:50:32 | ControlFlowNode for format_error() | test.py:49:15:49:36 | ControlFlowNode for Attribute() | test.py:50:16:50:32 | ControlFlowNode for format_error() | $@ may be exposed to an external user | test.py:49:15:49:36 | ControlFlowNode for Attribute() | Error information |

‎python/ql/test/query-tests/Security/CWE-209/Stacktrace.py‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
import sys, traceback
2+
3+
try:
4+
1/0
5+
except:
6+
exc_type, exc_value, exc_traceback = sys.exc_info() #$ exceptionInfo
7+
8+
tb = traceback.extract_tb(exc_traceback) #$ exceptionInfo
9+
stack = traceback.extract_stack() #$ exceptionInfo
10+
print(traceback.format_exc(1, tb)) #$ exceptionInfo
11+
print(traceback.format_exception(exc_type, exc_value, exc_traceback)) #$ exceptionInfo
12+
print(traceback.format_exception_only(None, exc_value)) #$ exceptionInfo
13+
print(traceback.format_list(stack)) #$ exceptionInfo
14+
print(traceback.format_stack()) #$ exceptionInfo
15+
print(traceback.format_tb(exc_traceback)) #$ exceptionInfo

‎python/ql/test/query-tests/Security/CWE-209/test.py‎

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,25 +12,41 @@ def do_computation():
1212
def server_bad():
1313
try:
1414
do_computation()
15-
except Exception as e:
16-
return traceback.format_exc()
15+
except Exception:
16+
return traceback.format_exc() #$ exceptionInfo
17+
18+
# BAD
19+
@app.route('/bad/direct')
20+
def server_bad_direct():
21+
try:
22+
do_computation()
23+
except Exception as e: #$ exceptionInfo
24+
return e
25+
26+
# BAD
27+
@app.route('/bad/traceback')
28+
def server_bad_traceback():
29+
try:
30+
do_computation()
31+
except Exception as e: #$ exceptionInfo
32+
return e.__traceback__
1733

1834
# GOOD
1935
@app.route('/good')
2036
def server_good():
2137
try:
2238
do_computation()
23-
except Exception as e:
24-
log(traceback.format_exc())
39+
except Exception:
40+
log(traceback.format_exc()) #$ exceptionInfo
2541
return "An internal error has occurred!"
2642

2743
#BAD
2844
@app.route('/bad/with-flow')
2945
def server_bad_flow():
3046
try:
3147
do_computation()
32-
except Exception as e:
33-
err = traceback.format_exc()
48+
except Exception:
49+
err = traceback.format_exc() #$ exceptionInfo
3450
return format_error(err)
3551

3652
def format_error(msg):

0 commit comments

Comments
 (0)