Bugzilla – Bug 704608
VUL-1: CVE-2011-2722: hplip: insecure tmp file handling in hpcupsfax.cpp -> potential read/write of arbitrary files
Last modified: 2019-05-01 15:58:42 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?)
CVE-2011-2722
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.
I have to specify a comment when changing the status of a bug from RESOLVED to REOPENED.
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.
That it can only be switched on by root is no excuse of writing /tmp files unsafe.
I think it is better when you discuss it directly with HPLIP upstream - e.g. on https://bugs.launchpad.net/hplip/+bug/809904
the lp url does not work. do you have a working url?
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.
I commented upstream. simplest patch would be to just remove this code for us, as we dont use it.
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.
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.
I think severity "minor" and priority "low" is suitable.
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.
Fixed for openSUSE:Factory via submitrequest 86531 "HPLIP version upgrade to 3.11.10" where the issue is fixed by upstream.
Johannes, could you also submit it for SLE 11 SP2?
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.
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)
releasing updates for SLE11-SP1 suffices (minor issue) -> done