`TM.check()` overwrites explicit Dataflow `protocol`, `dstPort` and `controls.isEncrypted` with sink defaults (regression in 1.4.0)

Abierto
  • ¿Algo no está bien?
#373 1 comentario 0 reacciones 0 asignados Ver en GitHub

Los mantenedores suelen responder en 3 días

@Svastikkka ya está trabajando en esto.

Desde el 29/10/2026.

  • #374 de @Svastikkka — abierto

Evaluación

Dificultad
3/5
Tiempo estimado
1-2 días
Aptitud para principiantes
75/100
Tipo de issue
Error
Claridad
Bien especificado
Estado de actividad
Activo
Stack tecnológico
python
Área
security, testing

Línea de trabajo

Start in pytm/pytm.py at _FlowDefaultsBuilder._apply_forward_defaults and _apply_response_defaults, then check how _safeset and Pydantic’s model_fields_set represent explicitly set fields. Reproduce the issue with the provided Python example and add regression tests for explicit forward and response flow values surviving TM.check(). Done means those values remain intact and the findings match the 1.3.1 results.

Escrito por el modelo de indexación a partir del texto del issue.

Descripción

Summary

Since 1.4.0, TM.check() replaces a dataflow's explicitly set protocol, dstPort and
controls.isEncrypted with the sink's values, which are usually the defaults ("", -1,
False). Threats are then evaluated against those values, so findings change. 1.3.1 kept
the explicit values.

Reproduction

pytm 1.4.0, Python 3.12:

from pytm import TM, Actor, Dataflow, Server, TLSVersion

tm = TM("repro")
tm.description = "Dataflow defaults repro"
user = Actor("User")
web = Server("Web")

flow = Dataflow(user, web, "Login")
flow.protocol = "HTTPS"
flow.dstPort = 443
flow.tlsVersion = TLSVersion.TLSv12
flow.controls.isEncrypted = True

print("before check:", repr(flow.protocol), flow.dstPort, flow.controls.isEncrypted)
tm.check()
print("after check: ", repr(flow.protocol), flow.dstPort, flow.controls.isEncrypted)
tm.resolve()
print("findings on the flow:", sorted(f.threat_id for f in tm.findings if f.target == "Login"))
After check() Findings on the flow
1.3.1 'HTTPS' 443 True AC05, CR06, DE01, DE03
1.4.0 '' -1 False AC05, CR06, CR08, DE01, DE03

CR08 (Client-Server Protocol Manipulation, not target.controls.isEncrypted or ...) is
reported only because isEncrypted was reset. Twelve threat conditions in threats.json
read protocol, dstPort or isEncrypted, so any model that sets these on flows can be
affected.

The bundled example tm.py shows it too: every flow loses its protocol and port after
check() (for example "Insert query with comments" goes from MySQL/3306 to ''/-1).

Cause

_FlowDefaultsBuilder._apply_forward_defaults (and _apply_response_defaults) in
pytm/pytm.py:

flow._safeset(
    "protocol", getattr(flow.sink, "protocol", getattr(flow, "protocol", ""))
)
flow._safeset(
    "dstPort", getattr(flow.sink, "port", getattr(flow, "dstPort", -1))
)

In 1.3.1, attributes were write-once var descriptors: assigning to an attribute that was
already set raised ValueError, which _safeset swallowed. Defaults therefore only filled
attributes the model left unset. In 1.4.0 the attributes are Pydantic fields with
validate_assignment=True, so _safeset always succeeds. And because every Pydantic
element has protocol and port, the getattr fallback to the flow's own value never
runs.

The same pattern resets isEncrypted from the sink's controls and, for responses,
protocol, srcPort and isEncrypted from the source.

Still present on master at 381dea0 (2026-10-05).

Suggested fix

Restore the 1.3.1 semantics: a default fills a field only if the model did not set it.
Pydantic already tracks this: model_fields_set includes fields given as constructor
keywords and fields assigned later.

def _fill(obj, attr, value):
    if attr not in obj.model_fields_set:
        obj._safeset(attr, value)

def _apply_forward_defaults(self, flow):
    _fill(flow, "protocol", flow.sink.protocol)
    _fill(flow, "dstPort", flow.sink.port)
    _fill(flow.controls, "isEncrypted", flow.sink.controls.isEncrypted)
    _fill(flow.controls, "authenticatesDestination",
          flow.source.controls.authenticatesDestination)
    _fill(flow.controls, "checksDestinationRevocation",
          flow.source.controls.checksDestinationRevocation)

and the same _fill in _apply_response_defaults. With the forward-defaults part of this
change applied to 1.4.0, the reproduction above gives 'HTTPS' 443 True and the 1.3.1
findings. A regression test asserting explicit flow values survive check() would catch
this in future.

Lenguaje dominante
Python
Estrellas
1.2k
Forks
237
Merge medio
4 d 2 h
PR fusionados (30 d)
9

Preparar el entorno

Abrir en Codespaces

Inicia el contenedor de desarrollo del proyecto en tu navegador, con tu propia cuenta de GitHub.

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de OWASP/pytm

Todos los issues de OWASP/pytm

Issues similares

Más issues de Python

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.