Ubuntu
software-properties package

ansi escape sequence injection in add-apt-repository

Bug #1890286 reported by Jason A. Donenfeld
258
This bug affects 1 person
Affects Status Importance Assigned to Milestone
software-properties (Ubuntu)
Fix Released
Undecided
Unassigned

Bug Description

This was reported to oss-security and to <email address hidden>, but I figure I should make a real bug report, as otherwise it'll probably be missed. Original post from https://www.openwall.com/lists/oss-security/2020/08/03/1 follows below.

--

Hi,

I've found a rather low grade concern: I'm able to inject ANSI escape
sequences into PPA descriptions on Launchpad, and then have them
rendered by add-apt-repository *before* the user consents to actually
adding that repository. There might be some sort of trust barrier
issue with that. This could be used to clear the screen and imitate a
fresh bash prompt, upload files, dump the current screen to a file, or
other classic shenanigans, well chronicled in the archives of oss-sec.

PoC time -- I'm using this "feature" for good at the moment to
announce the deprecation in bold text of a PPA that I maintain:
https://data.zx2c4.com/add-apt-repository-ansi-injection.png

The proper fix to this is likely to do sanitization on the
add-apt-repository side.

Regards,
Jason

CVE References

Revision history for this message
Jason A. Donenfeld (zx2c4) wrote :

Looks like this has come up before in other utilities and was fixed, such as https://bugs.launchpad.net/ubuntu/+source/base-files/+bug/1649352 .

information type: Private Security → Public
information type: Public → Public Security
summary: - ansi escape sequence injection into add-apt-repository
+ ansi escape sequence injection in add-apt-repository
Revision history for this message
Seth Arnold (seth-arnold) wrote :

Thanks Jason, please use CVE-2020-15709 for this issue.

Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package software-properties - 0.98.9.2

---------------
software-properties (0.98.9.2) focal-security; urgency=medium

  * SECURITY UPDATE: malicious repo could send ANSI sequences to terminal
    (LP: #1890286)
    - add-apt-repository: strip ANSI sequences from the description.
    - CVE-2020-15709

 -- Marc Deslauriers <email address hidden> Fri, 07 Aug 2020 09:15:34 -0400

Changed in software-properties (Ubuntu):
status: New → Fix Released
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package software-properties - 0.96.24.32.14

---------------
software-properties (0.96.24.32.14) bionic-security; urgency=medium

  * SECURITY UPDATE: malicious repo could send ANSI sequences to terminal
    (LP: #1890286)
    - add-apt-repository: strip ANSI sequences from the description.
    - CVE-2020-15709

 -- Marc Deslauriers <email address hidden> Fri, 07 Aug 2020 10:07:43 -0400

Changed in software-properties (Ubuntu):
status: New → Fix Released
Revision history for this message
Jason A. Donenfeld (zx2c4) wrote :

I'm not convinced that really cuts it. Namely, from the diff:

- print(" %s" % (info["description"] or ""))
+ # strip ANSI escape sequences
+ description = re.sub(r"(\x9B|\x1B\[)[0-?]*[ -/]*[@-~]",
+ "", info["description"] or "")
+
+ print(" %s" % description)

There are sequences that don't get filtered by that. Aside from the usual things like \r or \b, it looks like https://man7.org/linux/man-pages/man4/console_codes.4.html lists a few codes that defy it too. While that diff above might be the "stackoverflow answer", it doesn't seem complete.

Instead, why not just adopt a whitelist policy? Only allow visible and space characters, or something like that.

Revision history for this message
Marc Deslauriers (mdeslaur) wrote :

Hi,

Could you elaborate which codes in that manpage you feel are dangerous and are actually implemented by the common terminals? The old screendump and window title codes were disabled long ago, I'm not sure any of the others are anything other than a nuisance.

Revision history for this message
Jason A. Donenfeld (zx2c4) wrote :

You might be right that the remaining ones that slip through your regex are mere "nuisance"s. But you know how those things go - one man's nuisance is another man's vuln. Some of those, anyhow, are implemented by the Linux console driver.

Why not just take the tried and true "safe" route, as implemented by vis(3)'s VIS_SAFE or similar? Otherwise it sounds like you're playing with a bit of fire.

Put differently, is there some legitimate use case of the ANSI escape characters that make you want to preserve some of their usage while disallowing other parts? If so, that would really surprise me.

To post a comment you must log in.
This report contains Public Security information  
Everyone can see this security related information.
Subscribing...

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.