Skip to content

Commit 4772acf

Browse files
fix(iast): resolve PosixPath error in os.path patching [backport 3.3] (#13099)
Backport 303bae7 from #13085 to 3.3. This PR fixes an issue with PosixPath handling in IAST path operations. The changes: - Fix os.path patching errors in IAST taint tracking - Improve path operations in aspects.py - Add test coverage for os.path aspects ## Checklist - [x] PR author has checked that all the criteria below are met - The PR description includes an overview of the change - The PR description articulates the motivation for the change - The change includes tests OR the PR description describes a testing strategy - The PR description notes risks associated with the change, if any - Newly-added code is easy to change - The change follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Reviewer has checked that all the criteria below are met - Title is accurate - All changes are related to the pull request's stated goal - Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - Testing strategy adequately addresses listed risks - Newly-added code is easy to change - Release note makes sense to a user of the library - If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting) --------- Co-authored-by: Alberto Vara <[email protected]>
1 parent 2acc8b9 commit 4772acf

File tree

3 files changed

+210
-18
lines changed

3 files changed

+210
-18
lines changed

ddtrace/appsec/_iast/_taint_tracking/aspects.py

Lines changed: 81 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
from builtins import bytes as builtin_bytes
33
import codecs
44
import itertools
5+
import os
56
from re import Match
67
from re import Pattern
78
from types import BuiltinFunctionType
@@ -61,14 +62,6 @@
6162
add_inplace_aspect = aspects.add_inplace_aspect
6263
index_aspect = aspects.index_aspect
6364
modulo_aspect = aspects.modulo_aspect
64-
ospathbasename_aspect = _aspect_ospathbasename
65-
ospathdirname_aspect = _aspect_ospathdirname
66-
ospathjoin_aspect = _aspect_ospathjoin
67-
ospathnormcase_aspect = _aspect_ospathnormcase
68-
ospathsplit_aspect = _aspect_ospathsplit
69-
ospathsplitdrive_aspect = _aspect_ospathsplitdrive
70-
ospathsplitext_aspect = _aspect_ospathsplitext
71-
ospathsplitroot_aspect = _aspect_ospathsplitroot
7265
rsplit_aspect = _aspect_rsplit
7366
slice_aspect = aspects.slice_aspect
7467
split_aspect = _aspect_split
@@ -1263,3 +1256,83 @@ def re_expand_aspect(orig_function: Optional[Callable], flag_added_args: int, *a
12631256
iast_propagation_error_log(f"re_expand_aspect. {e}")
12641257

12651258
return result
1259+
1260+
1261+
def ospathjoin_aspect(*args: Any, **kwargs: Any) -> Any:
1262+
if all(isinstance(arg, IAST.TEXT_TYPES) for arg in args):
1263+
try:
1264+
return _aspect_ospathjoin(*args, **kwargs)
1265+
except Exception as e:
1266+
iast_propagation_error_log(f"ospathjoin_aspect. {e}")
1267+
1268+
return os.path.join(*args, **kwargs)
1269+
1270+
1271+
def ospathbasename_aspect(*args: Any, **kwargs: Any) -> Any:
1272+
if all(isinstance(arg, IAST.TEXT_TYPES) for arg in args):
1273+
try:
1274+
return _aspect_ospathbasename(*args, **kwargs)
1275+
except Exception as e:
1276+
iast_propagation_error_log(f"_aspect_ospathbasename. {e}")
1277+
1278+
return os.path.basename(*args, **kwargs)
1279+
1280+
1281+
def ospathdirname_aspect(*args: Any, **kwargs: Any) -> Any:
1282+
if all(isinstance(arg, IAST.TEXT_TYPES) for arg in args):
1283+
try:
1284+
return _aspect_ospathdirname(*args, **kwargs)
1285+
except Exception as e:
1286+
iast_propagation_error_log(f"_aspect_ospathdirname. {e}")
1287+
1288+
return os.path.dirname(*args, **kwargs)
1289+
1290+
1291+
def ospathnormcase_aspect(*args: Any, **kwargs: Any) -> Any:
1292+
if all(isinstance(arg, IAST.TEXT_TYPES) for arg in args):
1293+
try:
1294+
return _aspect_ospathnormcase(*args, **kwargs)
1295+
except Exception as e:
1296+
iast_propagation_error_log(f"ospathnormcase_aspect. {e}")
1297+
1298+
return os.path.normcase(*args, **kwargs)
1299+
1300+
1301+
def ospathsplit_aspect(*args: Any, **kwargs: Any) -> Any:
1302+
if all(isinstance(arg, IAST.TEXT_TYPES) for arg in args):
1303+
try:
1304+
return _aspect_ospathsplit(*args, **kwargs)
1305+
except Exception as e:
1306+
iast_propagation_error_log(f"ospathnormcase_aspect. {e}")
1307+
1308+
return os.path.split(*args, **kwargs)
1309+
1310+
1311+
def ospathsplitdrive_aspect(*args: Any, **kwargs: Any) -> Any:
1312+
if all(isinstance(arg, IAST.TEXT_TYPES) for arg in args):
1313+
try:
1314+
return _aspect_ospathsplitdrive(*args, **kwargs)
1315+
except Exception as e:
1316+
iast_propagation_error_log(f"_aspect_ospathsplitdrive. {e}")
1317+
1318+
return os.path.splitdrive(*args, **kwargs)
1319+
1320+
1321+
def ospathsplitext_aspect(*args: Any, **kwargs: Any) -> Any:
1322+
if all(isinstance(arg, IAST.TEXT_TYPES) for arg in args):
1323+
try:
1324+
return _aspect_ospathsplitext(*args, **kwargs)
1325+
except Exception as e:
1326+
iast_propagation_error_log(f"_aspect_ospathsplitext. {e}")
1327+
1328+
return os.path.splitext(*args, **kwargs)
1329+
1330+
1331+
def ospathsplitroot_aspect(*args: Any, **kwargs: Any) -> Any:
1332+
if all(isinstance(arg, IAST.TEXT_TYPES) for arg in args):
1333+
try:
1334+
return _aspect_ospathsplitroot(*args, **kwargs)
1335+
except Exception as e:
1336+
iast_propagation_error_log(f"_aspect_ospathsplitroot. {e}")
1337+
1338+
return os.path.splitroot(*args, **kwargs) # type: ignore[attr-defined]
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
fixes:
3+
- |
4+
Code Security: Fixed an issue with PosixPath handling in path operations that could cause
5+
errors during taint tracking. This fix improves stability and slightly reduces
6+
import times.

tests/appsec/iast/aspects/test_ospath_aspects.py

Lines changed: 123 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
import os
2+
from pathlib import PosixPath
23
import sys
34

5+
from hypothesis import given
6+
from hypothesis.strategies import text
47
import pytest
58

69
from ddtrace.appsec._iast._taint_tracking import OriginType
@@ -176,10 +179,130 @@ def test_ospathjoin_last_slash_tainted():
176179
assert get_tainted_ranges(res) == [TaintRange(0, 4, Source("test_ospath", "/bar", OriginType.PARAMETER))]
177180

178181

182+
@given(text())
183+
def test_ospathbasename_no_exceptions(string_):
184+
assert os.path.basename(PosixPath(string_)) == ospathbasename_aspect(PosixPath(string_))
185+
assert os.path.basename(string_) == ospathbasename_aspect(string_)
186+
187+
188+
def test_ospathbasename_wrong_arg():
189+
with pytest.raises(TypeError):
190+
_ = os.path.basename(42, "333")
191+
192+
with pytest.raises(TypeError):
193+
_ = os.path.basename(42)
194+
195+
with pytest.raises(TypeError):
196+
_ = os.path.basename(["a", "b", "c"])
197+
198+
with pytest.raises(TypeError):
199+
_ = ospathbasename_aspect(42, "333")
200+
201+
with pytest.raises(TypeError):
202+
_ = ospathbasename_aspect(42)
203+
204+
with pytest.raises(TypeError):
205+
_ = ospathbasename_aspect(["a", "b", "c"])
206+
207+
208+
@given(text())
209+
def test_ospathdirname_no_exceptions(string_):
210+
assert os.path.dirname(PosixPath(string_)) == ospathdirname_aspect(PosixPath(string_))
211+
assert os.path.dirname(string_) == ospathdirname_aspect(string_)
212+
213+
214+
def test_ospathdirname_wrong_arg():
215+
with pytest.raises(TypeError):
216+
_ = os.path.dirname(42, "333")
217+
218+
with pytest.raises(TypeError):
219+
_ = os.path.dirname(42)
220+
221+
with pytest.raises(TypeError):
222+
_ = os.path.dirname(["a", "b", "c"])
223+
224+
with pytest.raises(TypeError):
225+
_ = ospathdirname_aspect(42, "333")
226+
227+
with pytest.raises(TypeError):
228+
_ = ospathdirname_aspect(42)
229+
230+
with pytest.raises(TypeError):
231+
_ = ospathdirname_aspect(["a", "b", "c"])
232+
233+
234+
@given(text())
235+
def test_ospathjoin_no_exceptions(string_):
236+
assert os.path.join(PosixPath(string_), string_) == ospathjoin_aspect(PosixPath(string_), string_)
237+
assert os.path.join(PosixPath(string_)) == ospathjoin_aspect(PosixPath(string_))
238+
assert os.path.join(string_) == ospathjoin_aspect(string_)
239+
240+
179241
def test_ospathjoin_wrong_arg():
242+
def iterator_with_exception():
243+
for i in range(5):
244+
yield i
245+
raise ValueError("there is a problem in iterator_with_exception")
246+
247+
with pytest.raises(TypeError):
248+
_ = os.path.join("root", 42, "foobar")
249+
250+
with pytest.raises(TypeError):
251+
_ = os.path.join(("a", "b"))
252+
253+
with pytest.raises(TypeError):
254+
_ = os.path.join([PosixPath("a"), PosixPath("b")])
255+
256+
with pytest.raises(ValueError):
257+
_ = os.path.join("".join(iterator_with_exception()))
258+
259+
with pytest.raises(TypeError):
260+
_ = os.path.join([PosixPath("a"), PosixPath("b")])
261+
180262
with pytest.raises(TypeError):
181263
_ = ospathjoin_aspect("root", 42, "foobar")
182264

265+
with pytest.raises(TypeError):
266+
_ = ospathjoin_aspect(("a", "b"))
267+
268+
with pytest.raises(TypeError):
269+
_ = ospathjoin_aspect([PosixPath("a"), PosixPath("b")])
270+
271+
with pytest.raises(ValueError):
272+
_ = ospathjoin_aspect("".join(iterator_with_exception()))
273+
274+
275+
@given(text())
276+
def test_ospathnormcase_no_exceptions(string_):
277+
assert os.path.normcase(PosixPath(string_)) == ospathnormcase_aspect(PosixPath(string_))
278+
assert os.path.normcase(string_) == ospathnormcase_aspect(string_)
279+
280+
281+
@given(text())
282+
def test_ospathsplit_no_exceptions(string_):
283+
assert os.path.split(PosixPath(string_)) == ospathsplit_aspect(PosixPath(string_))
284+
assert os.path.split(string_) == ospathsplit_aspect(string_)
285+
286+
287+
@pytest.mark.skipif(sys.version_info < (3, 12) and os.name != "nt", reason="Requires Python 3.12 or Windows")
288+
@given(text())
289+
def test_ospathsplitdrive_no_exceptions(string_):
290+
assert os.path.splitdrive(PosixPath(string_)) == ospathsplitdrive_aspect(PosixPath(string_))
291+
assert os.path.splitdrive(string_) == ospathsplitdrive_aspect(string_)
292+
293+
294+
@given(text())
295+
def test_ospathsplitext_no_exceptions(string_):
296+
assert os.path.splitext(PosixPath(string_)) == ospathsplitext_aspect(PosixPath(string_))
297+
assert os.path.splitext(string_) == ospathsplitext_aspect(string_)
298+
299+
300+
@pytest.mark.skipif(sys.version_info < (3, 12), reason="Requires Python 3.12")
301+
@given(text())
302+
def test_ospathsplitroot_no_exceptions(string_):
303+
assert os.path.splitroot(PosixPath(string_)) == ospathsplitroot_aspect(PosixPath(string_))
304+
assert os.path.splitroot(string_) == ospathsplitroot_aspect(string_)
305+
183306

184307
def test_ospathjoin_bytes_nottainted():
185308
res = ospathjoin_aspect(b"nottainted", b"alsonottainted")
@@ -255,11 +378,6 @@ def test_ospathbasename_nottainted():
255378
assert not get_tainted_ranges(res)
256379

257380

258-
def test_ospathbasename_wrong_arg():
259-
with pytest.raises(TypeError):
260-
_ = ospathbasename_aspect(42)
261-
262-
263381
def test_ospathbasename_bytes_tainted():
264382
tainted_foobarbaz = taint_pyobject(
265383
pyobject=b"/foo/bar/baz",
@@ -403,11 +521,6 @@ def test_ospathdirname_nottainted():
403521
assert not get_tainted_ranges(res)
404522

405523

406-
def test_ospathdirname_wrong_arg():
407-
with pytest.raises(TypeError):
408-
_ = ospathdirname_aspect(42)
409-
410-
411524
def test_ospathdirname_bytes_tainted():
412525
tainted_foobarbaz = taint_pyobject(
413526
pyobject=b"/foo/bar/baz",

0 commit comments

Comments
 (0)