FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Python: Model `bytearray` construction and `take_bytes` by tausbn · Pull Request #22746 · github/codeql · GitHub

Repository navigation

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension .md  (1) .py  (1) .qll  (1) All 3 file types selected
Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* Added taint-flow modeling for `bytearray` construction and Python 3.15's `bytearray.take_bytes` method.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,8 @@ predicate subscriptStep(DataFlow::CfgNode nodeFrom, DataFlow::CfgNode nodeTo) {
}

/**
* Holds if taint can flow from `nodeFrom` to `nodeTo` with a step related to string
* manipulation.
* Holds if taint can flow from `nodeFrom` to `nodeTo` by manipulating string-like
* data, including text strings, byte strings, and byte arrays.
*
* Note that since we cannot easily distinguish when something is a string, this can
* also make taint flow on `<non string>.replace(foo, bar)`.
Expand All @@ -124,6 +124,18 @@ predicate stringManipulation(DataFlow::CfgNode nodeFrom, DataFlow::CfgNode nodeT
nodeFrom in [call.getArg(0), call.getArgByName("object")]
)
or
// Bytearray construction and byte extraction.
exists(DataFlow::CallCfgNode call | call = nodeTo |
call = API::builtin("bytearray").getACall() and
nodeFrom in [call.getArg(0), call.getArgByName("source")]
or
call.(DataFlow::MethodCallNode).calls(nodeFrom, "take_bytes")
or
// Unbound calls: bytearray.take_bytes(buffer).
call = API::builtin("bytearray").getMember("take_bytes").getACall() and
nodeFrom = call.getArg(0)
)
or
// String methods. Note that this doesn't recognize `meth = "foo".upper; meth()`
exists(CallNode call, string method_name, ControlFlowNode object |
call = nodeTo.getNode() and
Expand Down
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
# Add taintlib to PATH so it can be imported during runtime without any hassle
import sys; import os; sys.path.append(os.path.dirname(os.path.dirname((__file__))))
from taintlib import TAINTED_BYTES, TAINTED_STRING, ensure_tainted, ensure_not_tainted, taint


def constructors():
import builtins
from builtins import bytearray as make_buffer

ensure_tainted(
bytearray(TAINTED_BYTES), # $ tainted
bytearray(source=TAINTED_BYTES), # $ tainted
bytearray(TAINTED_STRING, "utf-8"), # $ tainted
bytearray(source=TAINTED_STRING, encoding="utf-8"), # $ tainted
builtins.bytearray(TAINTED_BYTES), # $ tainted
make_buffer(TAINTED_BYTES), # $ tainted
)
ensure_not_tainted(bytearray(b"safe"))


def shadowed_constructor():
def bytearray(source):
return b"safe"

ensure_not_tainted(bytearray(TAINTED_BYTES))


def take_bytes():
ensure_tainted(
bytearray(TAINTED_BYTES).take_bytes(), # $ tainted
bytearray(TAINTED_BYTES).take_bytes(None), # $ tainted
bytearray(source=TAINTED_STRING, encoding="utf-8").take_bytes().decode(), # $ tainted
bytearray.take_bytes(bytearray(TAINTED_BYTES)), # $ tainted
)

buffer = bytearray(TAINTED_BYTES)
take = buffer.take_bytes
ensure_tainted(take()) # $ tainted

ensure_not_tainted(bytearray(b"safe").take_bytes())
ensure_not_tainted(bytearray().take_bytes())


def take_partial_bytes():
buffer = bytearray(TAINTED_BYTES)
ensure_tainted(buffer.take_bytes(1)) # $ tainted
ensure_tainted(buffer) # $ tainted
ensure_tainted(buffer.take_bytes(-1)) # $ tainted
ensure_tainted(buffer) # $ tainted

size = 1
taint(size)
ensure_not_tainted(bytearray(b"safe").take_bytes(size))


def empty_results():
buffer = bytearray(TAINTED_BYTES)
# Whole-buffer taint does not distinguish empty slices.
ensure_not_tainted(buffer[:0]) # $ SPURIOUS: tainted
ensure_not_tainted(buffer.take_bytes(0)) # $ SPURIOUS: tainted


def consumed_buffer():
buffer = bytearray(b"abc")
taint(buffer)
result = buffer.take_bytes()
ensure_tainted(result) # $ tainted

# Whole-buffer taint is not removed when the buffer is emptied.
ensure_not_tainted(buffer) # $ SPURIOUS: tainted
ensure_not_tainted(buffer.take_bytes()) # $ SPURIOUS: tainted


def cleared_buffer():
buffer = bytearray(b"abc")
taint(buffer)
buffer.clear()
ensure_not_tainted(buffer) # $ SPURIOUS: tainted

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This could be fixed easily with a barrier model, right?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I think you're right. I'll add this as a follow-up PR.



constructors()
shadowed_constructor()
cleared_buffer()
if sys.version_info >= (3, 15):
take_bytes()
take_partial_bytes()
empty_results()
consumed_buffer()
Loading

Back | FazBrowse Home | New Git URL