Skip to content

Commit 498703f

Browse files
committed
Python: Escaping only valid with both input/output defined
Problematic part is ```codeql /** A escape from string format with `markupsafe.Markup` as the format string. */ private class MarkupEscapeFromStringFormat extends MarkupSafeEscape, Markup::StringFormat { override DataFlow::Node getAnInput() { result in [this.getArg(_), this.getArgByName(_)] and not result = Markup::instance() } override DataFlow::Node getOutput() { result = this } } ``` since the char-pred still holds even if `getAnInput` has no results... I will say that doing it this way feels kinda dirty, and we _could_ fix this by including the logic in `getAnInput` in the char-pred as well. But as I see it, that would just lead to a lot of code duplication, which isn't very nice.
1 parent 6539df6 commit 498703f

File tree

2 files changed

+7
-2
lines changed

2 files changed

+7
-2
lines changed

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,12 @@ module SqlExecution {
303303
class Escaping extends DataFlow::Node {
304304
Escaping::Range range;
305305

306-
Escaping() { this = range }
306+
Escaping() {
307+
this = range and
308+
// escapes that don't have _both_ input/output defined are not valid
309+
exists(range.getAnInput()) and
310+
exists(range.getOutput())
311+
}
307312

308313
/** Gets an input that will be escaped. */
309314
DataFlow::Node getAnInput() { result = range.getAnInput() }

python/ql/test/library-tests/frameworks/markupsafe/taint_test.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ def test():
3838
m_unsafe.format(SAFE), # $ escapeInput=SAFE escapeKind=html escapeOutput=m_unsafe.format(..) MISSING: tainted
3939
m_unsafe + ts, # $ escapeInput=ts escapeKind=html escapeOutput=BinaryExpr MISSING: tainted
4040

41-
m_safe.format(m_unsafe), # $ escapeKind=html escapeOutput=m_safe.format(..) MISSING: tainted
41+
m_safe.format(m_unsafe), # $ tainted
4242

4343
escape(ts).unescape(), # $ escapeInput=ts escapeKind=html escapeOutput=escape(..) MISSING: tainted
4444
escape_silent(ts).unescape(), # $ escapeInput=ts escapeKind=html escapeOutput=escape_silent(..) MISSING: tainted

0 commit comments

Comments
 (0)