Bug 704608 - (CVE-2011-2722) VUL-1: CVE-2011-2722: hplip: insecure tmp file handling in hpcupsfax.cpp -> potential read/write of arbitrary files
(CVE-2011-2722)
VUL-1: CVE-2011-2722: hplip: insecure tmp file handling in hpcupsfax.cpp -> p...
Status: RESOLVED FIXED
Classification: Novell Products
Product: SUSE Security Incidents
Classification: Novell Products
Component: General
unspecified
All SLED 11
: P4 - Low : Minor
: ---
Assigned To: Johannes Meixner
Security Team bot
maint:released:sle11-sp1:43762 CVSSv2...
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-07-08 11:21 UTC by Matthias Weckbecker
Modified: 2019-05-01 15:58 UTC (History)
4 users (show)

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


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Matthias Weckbecker 2011-07-08 11:21:22 UTC
422     FILE    *fp;
423     fp = NULL;
424     if (iLogLevel & SAVE_PCL_FILE)
425     {
426         fp = fopen ("/tmp/hpcupsfax.out", "w"); // <- VULN
427         system ("chmod 666 /tmp/hpcupsfax.out"); // <- "
428     }
429     while ((i = read (fdFax, pTmp, iSize)) > 0)
430     {
431         write (STDOUT_FILENO, pTmp, i);
432         if (iLogLevel & SAVE_PCL_FILE && fp)
433         {
434             fwrite (pTmp, 1, i, fp);
435         }
436     }
437     free (pTmp);

(Not public yet. Do we want to fix it?)
Comment 8 Matthias Weckbecker 2011-07-27 07:51:01 UTC
CVE-2011-2722
Comment 9 Swamp Workflow Management 2011-08-08 14:55:34 UTC
The SWAMPID for this issue is 42548.
This issue was rated as moderate.
Please submit fixed packages until 2011-08-22.
When done, please reassign the bug to security-team@suse.de.
Patchinfo will be handled by security team.
Comment 16 Johannes Meixner 2011-08-16 12:09:50 UTC
I have to specify a comment when changing the status of a bug
from RESOLVED to REOPENED.
Comment 17 Johannes Meixner 2011-08-16 12:13:35 UTC
Here Sanjay Kumar's reply on
https://bugs.launchpad.net/hplip/+bug/809904
-----------------------------------------------------------------------
Hi,

Sorry for the delay in response. Regarding the issue mentioned
by you, here are my comments.

1)
SAVE_PCL_FILE is a macro defined in ./prnt/hpcups/CommonDefinitions.h
of HPLIP Source code. ie
#define BASIC_LOG 1
#define SAVE_PCL_FILE 2
#define SAVE_INPUT_RASTERS 4
#define SEND_TO_PRINTER_ALSO 8

2)
iLogLevel is read from the file /etc/cups/cupsd.conf if following
line is present in the file.

hpLogLevel <NUMBER>

Where <NUMBER> is the integer value betwwen 1-15 based on our need.
Note: This line is not present in the above file by default.
For debugging purpose only root can add this line
in /etc/cups/cupsd.conf and then collect the logs.

Therefore "if (iLogLevel & SAVE_PCL_FILE) " condition will be True
only when /etc/cups/cupsd.conf file is modified in root mode and
then cups is restarted. Since "iLogLevel & SAVE_PCL_FILE" is true
only under special circumstances it should not be a security bug.

Thanks,
Sanjay

Changed in hplip:
status: 	New → Invalid 
-----------------------------------------------------------------------

I agree with Sanjay Kumar and close this bug report
as "invalid " too.
Comment 18 Marcus Meissner 2011-08-16 12:40:17 UTC
That it can only be switched on by root is no excuse of writing /tmp files unsafe.
Comment 19 Johannes Meixner 2011-08-16 12:51:19 UTC
I think it is better when you discuss it
directly with HPLIP upstream - e.g. on
https://bugs.launchpad.net/hplip/+bug/809904
Comment 22 Marcus Meissner 2011-08-16 13:01:19 UTC
the lp url does not work. do you have a working url?
Comment 23 Johannes Meixner 2011-08-16 13:25:26 UTC
Because I had filed
https://bugs.launchpad.net/hplip/+bug/809904
as a security bug, it was visible only to its direct subscribers.

But now - since it is no longer a security bug - I made it public.
Comment 24 Marcus Meissner 2011-08-16 15:38:14 UTC
I commented upstream.

simplest patch would be to just remove this code for us, as we dont use it.
Comment 25 Johannes Meixner 2011-08-17 08:03:17 UTC
From my point of view "/tmp/hpcupsfax.out" is not meant
as a temporary file but as output file for debugging purpose
which (unfortunately) exists in a directory (/tmp)
where any user can create a symbolic link like for example
  /tmp/hpcupsfax.out -> /etc/passwd
and then when
  system ("chmod 666 /tmp/hpcupsfax.out")
would be run as root (I don't know under which user it runs)
it would do an evil thing.

When "/tmp/hpcupsfax.out" is meant as output file for debugging purpose
it would be neither good to remove this debugging feature from HPLIP
nor would it be nice when the debugging output file name is not
a fixed name which is known in advance but instead it would be some
awkward "mktemp" name e.g. /tmp/hpcupsfax.out.XXXXXXXXXX

When "/tmp/hpcupsfax.out" is meant as output file for debugging purpose
it should be o.k. to remove an existing file or symbolic link
with this name via something like:

  if (iLogLevel & SAVE_PCL_FILE)
  {
    if (system ("rm -f /tmp/hpcupsfax.out"))
    { return 1;
    }
    fp = fopen ("/tmp/hpcupsfax.out", "w");
    system ("chmod 666 /tmp/hpcupsfax.out");
  }

But because of the stick bit in the /tmp/ directory
"rm -f /tmp/hpcupsfax.out" works only for root and for the user
who had created /tmp/hpcupsfax.out

I think this could be o.k. because when the stuff is run as root
it would enforce "the right thing" and when it is run as non-root
it would do "the right thing" when /tmp/hpcupsfax.out from
the same user already exists and otherwise it would return
something like a "failed" state as far as I guess the meaning
of "return 1" in prnt/hpijs/hpcupsfax.cpp

But I am not a security expert to finally decide about it.
Comment 26 Johannes Meixner 2011-08-17 08:17:47 UTC
Marcus,
many thanks for your comment at the upstream bug
https://bugs.launchpad.net/hplip/+bug/809904/comments/4

Because "there is no way to 'secure' enable this option
on systems with potential malicious users" I now understand
that it is actually a security issue.

  Debugging should also be possible in a secure way
  even on systems with potential malicious users.
Comment 27 Johannes Meixner 2011-08-17 08:21:15 UTC
I think severity "minor" and priority "low" is suitable.
Comment 28 Matthias Weckbecker 2011-08-17 08:23:54 UTC
I agree with you, Johannes. I think that was my fault. I added it as severity "normal" which is indeed too much. Thanks for correction.
Comment 29 Johannes Meixner 2011-10-05 06:53:35 UTC
Fixed for openSUSE:Factory via submitrequest 86531
"HPLIP version upgrade to 3.11.10"
where the issue is fixed by upstream.
Comment 30 Stefan Behlert 2011-10-06 08:11:46 UTC
Johannes, could you also submit it for SLE 11 SP2?
Comment 32 Swamp Workflow Management 2011-10-19 16:27:53 UTC
The SWAMPID for this issue is 43761.
This issue was rated as low.
Please submit fixed packages until 2011-11-16.
When done, please reassign the bug to security-team@suse.de.
Patchinfo will be handled by security team.
Comment 33 Swamp Workflow Management 2011-10-25 17:06:28 UTC
Update released for: hplip, hplip-debuginfo, hplip-debugsource, hplip-hpijs
Products:
SLE-DEBUGINFO 11-SP1 (i386, ia64, ppc64, s390x, x86_64)
SLE-DESKTOP 11-SP1 (i386, x86_64)
SLE-SERVER 11-SP1 (i386, ia64, ppc64, s390x, x86_64)
SLE-SERVER 11-SP1-TERADATA (x86_64)
SLES4VMWARE 11-SP1 (i386, x86_64)
Comment 34 Sebastian Krahmer 2011-10-26 11:40:39 UTC
releasing updates for SLE11-SP1 suffices (minor issue) -> done