naruto-lgtm commented on code in PR #68662:
URL: https://github.com/apache/airflow/pull/68662#discussion_r3523396518
##########
airflow-core/src/airflow/serialization/serialized_objects.py:
##########
@@ -662,6 +662,12 @@ def deserialize(cls, encoded_var: Any) -> Any:
exc_cls = import_string(exc_cls_name)
else:
exc_cls = import_string(f"builtins.{exc_cls_name}")
+ # ``exc_cls_name`` comes from the serialized payload. A tampered
blob can name any
+ # importable callable (``os.system``, ``builtins.exec``, ...)
which would then be
+ # invoked with attacker-supplied args. Only accept genuine
exception types, matching
+ # the import-path guards the timetable/window/wait-policy decoders
already apply.
+ if not (isinstance(exc_cls, type) and issubclass(exc_cls,
BaseException)):
Review Comment:
Moot now. Pushed eef021eb, which drops the whole exception branch from
BaseSerialization, so there is no import-then-validate ordering left to worry
about and nothing legitimate serializes an exception through the general
framework anymore.
##########
airflow-core/src/airflow/serialization/serialized_objects.py:
##########
@@ -658,14 +659,18 @@ def deserialize(cls, encoded_var: Any) -> Any:
args = deser["args"]
kwargs = deser["kwargs"]
del deser
+ # ``exc_cls_name`` comes from the serialized payload, so resolve
it without handing an
+ # attacker-controlled string to ``import_string`` -- that would
execute the named
+ # module's top-level code before any validation. The encode side
only ever emits
+ # ``airflow.*`` AirflowException subclasses for AIRFLOW_EXC_SER
and builtin
+ # KeyError/AttributeError for BASE_EXC_SER, so reject anything
outside those up front,
+ # mirroring the import-path guards in the
timetable/window/wait-policy decoders.
if type_ == DAT.AIRFLOW_EXC_SER:
+ if not exc_cls_name.startswith("airflow."):
Review Comment:
Gone in eef021eb. Rather than trying to make the prefix safe, I removed the
encode and decode branches entirely, so there is no import_string call on a
payload name to defend anymore.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
