Vamsi-klu commented on PR #69435:
URL: https://github.com/apache/airflow/pull/69435#issuecomment-4912281464
Confirmed the root cause and the fix. build_mime_message and
SmtpHook._build_mime_message both built Content-Disposition via an f-string, so
a filename containing a double quote (e.g. report.txt"; x-evil="1) closes the
quoted value and injects an extra parameter. Switching to
part.add_header("Content-Disposition", "attachment", filename=basename) hands
escaping to the stdlib. Ordinary names come out byte-identical to the old
output (attachment; filename="report.txt"), names with a quote get
backslash-escaped inside the quotes with no injected param, and non-ASCII names
are RFC 2231 encoded; get_filename() round-trips in every case. Both call sites
are fixed and each has a regression test that fails against the old f-string
(asserts get_filename() equals the raw name and x-evil is not in the parsed
params). I also checked the imap provider hook, which showed up in a search for
the same pattern, but it only parses received Content-Disposition headers, so
there is no third site to fi
x. One nit: the description says RFC 2231-encoded, but for ASCII-with-quote
inputs it is actually quoted-string backslash escaping (RFC 2231 only kicks in
for non-ASCII) -- same security result, just worth correcting in the summary.
Practical severity is limited since attachment filenames are usually
operator/DAG-author controlled, so this is hardening more than a high-severity
exploit. LGTM.
--
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]
