Bug 1214399 - VUL-0: hplip: use of fixed temporary paths in hppsfilter.c
Summary: VUL-0: hplip: use of fixed temporary paths in hppsfilter.c
Status: IN_PROGRESS
Alias: None
Product: openSUSE Tumbleweed
Classification: openSUSE
Component: Security (show other bugs)
Version: Current
Hardware: Other Other
: P3 - Medium : Normal (vote)
Target Milestone: ---
Assignee: Security Team bot
QA Contact: E-mail List
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-08-18 13:03 UTC by Matthias Gerstner
Modified: 2024-01-04 10:26 UTC (History)
6 users (show)

See Also:
Found By: ---
Services Priority:
Business Priority:
Blocker: ---
Marketing QA Status: ---
IT Deployment: ---


Attachments
suggested patch (6.63 KB, patch)
2023-09-08 08:46 UTC, Matthias Gerstner
Details | Diff
new version of the patch using atexit() (6.83 KB, patch)
2023-09-15 11:07 UTC, Matthias Gerstner
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Matthias Gerstner 2023-08-18 13:03:56 UTC
+++ This bug was initially created as a clone of Bug #1213332

The program /usr/lib/cups/filter/hpps has some insecure fixed /tmp path use,
found in prnt/hpps/hppsfilter.c:

    prnt/hpps/hppsfilter.c: sprintf(booklet_filename, "/tmp/%s.ps","booklet");
    prnt/hpps/hppsfilter.c: sprintf(temp_filename, "/tmp/%s.ps","temp");
    prnt/hpps/hppsfilter.c: sprintf(Nup_filename, "/tmp/%s.ps","NUP");

These are only used if "booklet printing" is enabled. The logic can be forced
by invoking the program like this:

    /usr/lib/cups/filter/hpps some-job some-user some-title 10 HPBookletFilter=10,fitplot,Duplex=DuplexTumble,number-up=1

The program will expect data to print on stdin this way. Just typing in some
garbage data and pressing ctrl-d will make it continue. There's a chance that
it will segfault, since error returns are largely not checked in this program.

There is also a `chmod()` on the /tmp/temp.ps file:

    hppsfilter.c:110 chmod(temp_filename, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);

The data to print (from stdin) is written to this file, and the file is also
made world readable explicitly using this chmod(). The issues with this are
multifold:

    - there is an information leak, since the print job data will become visible
      to everybody in the system.
    - there is violated data integrity, since other users can pre-create this
      file and manipulate the data to print.
    - it may allow to create files in unexpected places if the kernel's symlink
      protection is not around.
    - it may allow further unspecified impact if crafted data is placed into
      /tmp/temp.ps which is processed by the complex `PS_Booklet` function.

I will not research further if this could lead to code execution. The issue is
bad enough as it is.

All three temporary file paths need to be replaced by safely created temporary
files. I will approach upstream about this.
Comment 6 Matthias Gerstner 2023-08-21 14:07:59 UTC
All right I shared the information on the launchpad bug tracker. I tagged it
as security, which results in a private bug:

    https://bugs.launchpad.net/hplip/+bug/2032375

If some of you want to be added to the bug then tell me so, I should be able
to add you.

Supposedly this involves also some HP security people so lets see whether
there is a timely reaction. Should nothing happen within two weeks then we can
push a bit harder an publish at our own discretion if this still doesn't help.
Comment 18 Matthias Gerstner 2023-09-08 08:50:39 UTC
When working actively with this code base it shows that these tmp file issues
are only the worst part of its problems. That code is really in a sorry state,
the compiler warnings alone are a mess.

Anyway you can find a patch for the tmp file issues in attachment 869377 [details]. It's
against the openSUSE:Factory version but I have hopes that the patch also
applies to older versions. The patch compiles and I tested the patched Factory
version on Tumbleweed a bit. The safe tmp file paths are correctly used and
deleted again, and the program didn't crash any more often than it did before
applying the patch.
Comment 19 Martin Wilck 2023-09-14 15:56:23 UTC
(In reply to Matthias Gerstner from comment #18)
> When working actively with this code base it shows that these tmp file issues
> are only the worst part of its problems. That code is really in a sorry
> state,
> the compiler warnings alone are a mess.

No objections from my side.

> Anyway you can find a patch for the tmp file issues in attachment 869377 [details]
> [details]. It's
> against the openSUSE:Factory version but I have hopes that the patch also
> applies to older versions. The patch compiles and I tested the patched
> Factory
> version on Tumbleweed a bit. The safe tmp file paths are correctly used and
> deleted again, and the program didn't crash any more often than it did before
> applying the patch.

Thanks. The patch looks good to me. One nitpick: You could consider using atexit(cleanup_tempfiles()) to make sure no exit path is overlooked.
Comment 20 Matthias Gerstner 2023-09-15 11:07:16 UTC
Created attachment 869534 [details]
new version of the patch using atexit()
Comment 21 Matthias Gerstner 2023-09-15 11:07:42 UTC
> Thanks. The patch looks good to me. One nitpick: You could consider using
> atexit(cleanup_tempfiles()) to make sure no exit path is overlooked.

I attached a new version of the patch that has this feature.
Comment 22 Martin Wilck 2023-09-15 15:00:34 UTC
Thanks. IMO if you use atexit(), you don't need to all clean_tempfiles() from anywhere else. But as you've programmed the function such that it an be invoked multiple times without issues, I don't think that's a problem.
Comment 27 Matthias Gerstner 2023-11-15 13:36:08 UTC
Sorry for the continued silence. The 90 days maximum embargo time we offer
will be reached this week so I will publish the finding by the end of the week
no matter what.

You can already submit updates in IBS and prepare updates for OBS.

I suggest to publish on Friday 2023-11-17.
Comment 28 Martin Wilck 2023-11-15 20:22:47 UTC
Do we have a CVE number for our changelogs?
Comment 29 Martin Wilck 2023-11-15 21:36:35 UTC
I've prepared submissions for SLE15-SP2, SP3, SP4. I didn't submit them yet because of the missing CVE number. I have submitted the current factory package to SLE15-SP6, and will add the patch there once it hits factory.

hplip 3.16.11 in SLE12-SP5 doesn't support booklet mode yet, and thus doesn't need this patch.
Comment 30 Matthias Gerstner 2023-11-16 08:03:54 UTC
(In reply to martin.wilck@suse.com from comment #28)
> Do we have a CVE number for our changelogs?

No there is no CVE, because HP upstream is a Mitre CNA and we are not allowed
to circumvent them. Since they don't react there's nothing we can do. The bsc#
reference will have to suffice.
Comment 34 Matthias Gerstner 2023-11-17 10:18:56 UTC
I published the full report on the oss-security mailing list:

https://www.openwall.com/lists/oss-security/2023/11/17/1

Thanks for working on the updates. If all updates have been submitted then please reassign the bug to security-team@suse.de. My colleagues will take care of closing the bug once all updates are released.
Comment 36 Martin Wilck 2023-11-17 10:41:54 UTC
https://build.opensuse.org/request/show/1127274
SLE-15-SP6: https://build.suse.de/request/show/313016

Reassigning to security team per comment 34.
Comment 37 Maintenance Automation 2023-12-11 20:36:18 UTC
SUSE-SU-2023:4710-1: An update that has one security fix can now be installed.

Category: security (moderate)
Bug References: 1214399
Sources used:
Basesystem Module 15-SP5 (src): hplip-3.21.10-150400.3.11.1
Desktop Applications Module 15-SP4 (src): hplip-3.21.10-150400.3.11.1
Desktop Applications Module 15-SP5 (src): hplip-3.21.10-150400.3.11.1
openSUSE Leap 15.4 (src): hplip-3.21.10-150400.3.11.1
openSUSE Leap 15.5 (src): hplip-3.21.10-150400.3.11.1
Basesystem Module 15-SP4 (src): hplip-3.21.10-150400.3.11.1

NOTE: This line indicates an update has been released for the listed product(s). At times this might be only a partial fix. If you have questions please reach out to maintenance coordination.
Comment 38 Martin Wilck 2024-01-03 16:10:03 UTC
upstream has silently added this patch in 3.23.12.