Skip to content

Commit 347c890

Browse files
committed
Python: Model bytearray construction and take_bytes
We model these as string-like taint preserving steps.
1 parent 74211cc commit 347c890

3 files changed

Lines changed: 106 additions & 2 deletions

File tree

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Added taint-flow modeling for `bytearray` construction and Python 3.15's `bytearray.take_bytes` method.

‎python/ql/lib/semmle/python/dataflow/new/internal/TaintTrackingPrivate.qll‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,8 +107,8 @@ predicate subscriptStep(DataFlow::CfgNode nodeFrom, DataFlow::CfgNode nodeTo) {
107107
}
108108

109109
/**
110-
* Holds if taint can flow from `nodeFrom` to `nodeTo` with a step related to string
111-
* manipulation.
110+
* Holds if taint can flow from `nodeFrom` to `nodeTo` by manipulating string-like
111+
* data, including text strings, byte strings, and byte arrays.
112112
*
113113
* Note that since we cannot easily distinguish when something is a string, this can
114114
* also make taint flow on `<non string>.replace(foo, bar)`.
@@ -124,6 +124,18 @@ predicate stringManipulation(DataFlow::CfgNode nodeFrom, DataFlow::CfgNode nodeT
124124
nodeFrom in [call.getArg(0), call.getArgByName("object")]
125125
)
126126
or
127+
// Bytearray construction and byte extraction.
128+
exists(DataFlow::CallCfgNode call | call = nodeTo |
129+
call = API::builtin("bytearray").getACall() and
130+
nodeFrom in [call.getArg(0), call.getArgByName("source")]
131+
or
132+
call.(DataFlow::MethodCallNode).calls(nodeFrom, "take_bytes")
133+
or
134+
// Unbound calls: bytearray.take_bytes(buffer).
135+
call = API::builtin("bytearray").getMember("take_bytes").getACall() and
136+
nodeFrom = call.getArg(0)
137+
)
138+
or
127139
// String methods. Note that this doesn't recognize `meth = "foo".upper; meth()`
128140
exists(CallNode call, string method_name, ControlFlowNode object |
129141
call = nodeTo.getNode() and
Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
# Add taintlib to PATH so it can be imported during runtime without any hassle
2+
import sys; import os; sys.path.append(os.path.dirname(os.path.dirname((__file__))))
3+
from taintlib import TAINTED_BYTES, TAINTED_STRING, ensure_tainted, ensure_not_tainted, taint
4+
5+
6+
def constructors():
7+
import builtins
8+
from builtins import bytearray as make_buffer
9+
10+
ensure_tainted(
11+
bytearray(TAINTED_BYTES), # $ tainted
12+
bytearray(source=TAINTED_BYTES), # $ tainted
13+
bytearray(TAINTED_STRING, "utf-8"), # $ tainted
14+
bytearray(source=TAINTED_STRING, encoding="utf-8"), # $ tainted
15+
builtins.bytearray(TAINTED_BYTES), # $ tainted
16+
make_buffer(TAINTED_BYTES), # $ tainted
17+
)
18+
ensure_not_tainted(bytearray(b"safe"))
19+
20+
21+
def shadowed_constructor():
22+
def bytearray(source):
23+
return b"safe"
24+
25+
ensure_not_tainted(bytearray(TAINTED_BYTES))
26+
27+
28+
def take_bytes():
29+
ensure_tainted(
30+
bytearray(TAINTED_BYTES).take_bytes(), # $ tainted
31+
bytearray(TAINTED_BYTES).take_bytes(None), # $ tainted
32+
bytearray(source=TAINTED_STRING, encoding="utf-8").take_bytes().decode(), # $ tainted
33+
bytearray.take_bytes(bytearray(TAINTED_BYTES)), # $ tainted
34+
)
35+
36+
buffer = bytearray(TAINTED_BYTES)
37+
take = buffer.take_bytes
38+
ensure_tainted(take()) # $ tainted
39+
40+
ensure_not_tainted(bytearray(b"safe").take_bytes())
41+
ensure_not_tainted(bytearray().take_bytes())
42+
43+
44+
def take_partial_bytes():
45+
buffer = bytearray(TAINTED_BYTES)
46+
ensure_tainted(buffer.take_bytes(1)) # $ tainted
47+
ensure_tainted(buffer) # $ tainted
48+
ensure_tainted(buffer.take_bytes(-1)) # $ tainted
49+
ensure_tainted(buffer) # $ tainted
50+
51+
size = 1
52+
taint(size)
53+
ensure_not_tainted(bytearray(b"safe").take_bytes(size))
54+
55+
56+
def empty_results():
57+
buffer = bytearray(TAINTED_BYTES)
58+
# Whole-buffer taint does not distinguish empty slices.
59+
ensure_not_tainted(buffer[:0]) # $ SPURIOUS: tainted
60+
ensure_not_tainted(buffer.take_bytes(0)) # $ SPURIOUS: tainted
61+
62+
63+
def consumed_buffer():
64+
buffer = bytearray(b"abc")
65+
taint(buffer)
66+
result = buffer.take_bytes()
67+
ensure_tainted(result) # $ tainted
68+
69+
# Whole-buffer taint is not removed when the buffer is emptied.
70+
ensure_not_tainted(buffer) # $ SPURIOUS: tainted
71+
ensure_not_tainted(buffer.take_bytes()) # $ SPURIOUS: tainted
72+
73+
74+
def cleared_buffer():
75+
buffer = bytearray(b"abc")
76+
taint(buffer)
77+
buffer.clear()
78+
ensure_not_tainted(buffer) # $ SPURIOUS: tainted
79+
80+
81+
constructors()
82+
shadowed_constructor()
83+
cleared_buffer()
84+
if sys.version_info >= (3, 15):
85+
take_bytes()
86+
take_partial_bytes()
87+
empty_results()
88+
consumed_buffer()

0 commit comments

Comments
 (0)