Bug#718682: liblcms1: Buffer overflows in Little CMS v1.19
Thanks again for the feedback Alan. I have uploaded the newer version of the patch to the redhat bugzilla https://bugzilla.redhat.com/show_bug.cgi?id=991757#attach_783274 I had to create an intermediate buffer...
Bug#718682: liblcms1: Buffer overflows in Little CMS v1.19
Thanks Sebastian.
Shameful that to fix one I introduced another...
Regards
Pedro
On Aug 4, 2013 11:08 AM, Sebastian Ramacher sramac...@debian.org wrote:
Hi Pedro,
thank you for reporting this security issue.
On 2013-08-04 10:35:46, Pedro R wrote:
diff -urb lcms-1.19.dfsg/samples/icctrans.c
lcms-1.19.dfsg-patched/samples/icctrans.c
--- lcms-1.19.dfsg/samples/icctrans.c 2009-10-30 15:57:45.0 +
+++ lcms-1.19.dfsg-patched/samples/icctrans.c 2013-08-04
10:31:36.608445149 +0100
@@ -500,7 +500,7 @@
Prefix[0] = 0;
if (!lTerse)
-sprintf(Prefix, %s=, C);
+snprintf(Prefix, 20, %s=, C);
if (InHexa)
{
@@ -648,7 +648,9 @@
static
void GetLine(char* Buffer)
{
-scanf(%s, Buffer);
+size_t Buffer_size = sizeof(Buffer);
+fgets(Buffer, (Buffer_size - 1), stdin);
+sscanf(%s, Buffer);
This sscanf call is wrong and introduces a format string vulnerability.
sscanf's signature is int sscanf(const char* str, const char* fmt, ...)
where str is used as input and format is the second argument.
Regards
--
Sebastian Ramacher
Bug#718682: liblcms1: Buffer overflows in Little CMS v1.19
Hi Sebastian, sorry again for that fail. Here is the correct patch. Regards, Pedro lcms-1.19-b0f-v2.patch Description: Binary data
Bug#718682: liblcms1: Buffer overflows in Little CMS v1.19
Thanks for that Alan - I had no idea, and have been looking at lots of C
code lately that has probably has the same mistakes. I will keep an eye on
that.
Ok this patch is turning into a trainwreck - to everyone please be careful
when applying it.
Actually my original idea was more to point to the vulnerabilities that to
actually provide a working patch, but since lcms1 is not maintained
actively any more I decided to produce this. I guess in the future I will
say any patches I send are provided only an example and should not be
applied direclty..
Regards,
Pedro
Kind regards,
*Pedro Ribeiro*
Information Security Consultant
Professional Bug Hunter
On 6 August 2013 00:35, Alan Coopersmith alan.coopersm...@oracle.comwrote:
void GetLine(char* Buffer)
{
-scanf(%s, Buffer);
+size_t Buffer_size = sizeof(Buffer);
+fgets(Buffer, (Buffer_size - 1), stdin);
+sscanf(Buffer,%s);
sizeof() in the C language does not reach through a pointer to find the
size of
the underlying object - that code will always set Buffer_size to the size
of
the pointer itself (4 bytes on 32-bit, 8 bytes on 64-bit), not the size of
the
buffer the pointer is pointing to.
[Noticed when someone suggested we apply the patch from Debian to our
packages
as well.]
--
-Alan Coopersmith- alan.coopersm...@oracle.com
Oracle Solaris Engineering - http://blogs.oracle.com/alanc
Bug#718682: liblcms1: Buffer overflows in Little CMS v1.19
void GetLine(char* Buffer)
{
-scanf(%s, Buffer);
+size_t Buffer_size = sizeof(Buffer);
+fgets(Buffer, (Buffer_size - 1), stdin);
+sscanf(Buffer,%s);
sizeof() in the C language does not reach through a pointer to find the size of
the underlying object - that code will always set Buffer_size to the size of
the pointer itself (4 bytes on 32-bit, 8 bytes on 64-bit), not the size of the
buffer the pointer is pointing to.
[Noticed when someone suggested we apply the patch from Debian to our packages
as well.]
--
-Alan Coopersmith- alan.coopersm...@oracle.com
Oracle Solaris Engineering - http://blogs.oracle.com/alanc
--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#718682: liblcms1: Buffer overflows in Little CMS v1.19
On 08/ 5/13 05:00 PM, Pedro Ribeiro wrote: Thanks for that Alan - I had no idea, and have been looking at lots of C code lately that has probably has the same mistakes. I will keep an eye on that. More details/deeper explanations of when sizeof can and cannot work can be found in: https://www.securecoding.cert.org/confluence/display/seccode/ARR01-C.+Do+not+apply+the+sizeof+operator+to+a+pointer+when+taking+the+size+of+an+array https://en.wikibooks.org/wiki/C_Programming/Pointers_and_arrays#sizeof -- -Alan Coopersmith- alan.coopersm...@oracle.com Oracle Solaris Engineering - http://blogs.oracle.com/alanc -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#718682: liblcms1: Buffer overflows in Little CMS v1.19
On 08/ 5/13 04:35 PM, Alan Coopersmith wrote: +fgets(Buffer, (Buffer_size - 1), stdin); +sscanf(Buffer,%s); Oops, forgot to mention the sscanf is still wrong in this second revision. This code now reads a line from stdin and writes it to Buffer. The sscanf now takes Buffer as input, looks for a string pattern matching %s and writes it to, well, whatever the random uninitialized value is next on the stack, because there is no output argument provided for the %s. Fortunately, gcc -Wformat should find this and error out before anyone ships it. Of course, since this is used in a function that expects the string to be returned in Buffer, having sscanf read from Buffer and write the result somewhere else is also counterproductive. -- -Alan Coopersmith- alan.coopersm...@oracle.com Oracle Solaris Engineering - http://blogs.oracle.com/alanc -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#718682: liblcms1: Buffer overflows in Little CMS v1.19
Package: liblcms1
Version: 1.19
Severity: grave
Tags: upstream security patch
Justification: user security hole
I have found three (lame) buffer overflows in lcms-1.19. The problem lies in
the use of dangerous functions like scanf and sprintf to handle user input.
I have contacted the Little CMS developer and his answer was that people
concerned about security should update to Little CMS v2. To be honest I think
it's a reasonable answer since he has stopped supporting lcms-1 in 2009.
However this appears to be a package that is still widely in use in several
distributions, and included in other software as a library.
I am attaching patches here to address the issue. These have been compile
tested but I did not do any test beyond that. Please note that I am sending
this via a mobile device and the patches might be mangled (hopefully not).
If you have any questions please contact me back. If you do issue an advisory,
please credit Pedro Ribeiro (ped...@gmail.com).
Note that I have contacted the security team and was instructed to report this
bug here.
Regards,
Pedro
-- System Information:
Debian Release: 7.1
APT prefers stable
APT policy: (750, 'stable'), (650, 'testing'), (600, 'unstable'), (550,
'experimental')
Architecture: amd64 (x86_64)
Foreign Architectures: i386
Kernel: Linux 3.7.1-botto-secfixes3-grsec+ (SMP w/2 CPU cores; PREEMPT)
Locale: LANG=en_GB.UTF-8, LC_CTYPE=en_GB.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/bash
Only in lcms-1.19.dfsg-patched/: config.log
Only in lcms-1.19.dfsg-patched/: config.status
diff -urb lcms-1.19.dfsg/include/icc34.h lcms-1.19.dfsg-patched/include/icc34.h
--- lcms-1.19.dfsg/include/icc34.h 2013-08-04 10:20:43.0 +0100
+++ lcms-1.19.dfsg-patched/include/icc34.h 2013-08-04 10:27:21.746631360 +0100
@@ -151,15 +151,15 @@
PACKAGE_NAME is defined if autoconf is being used
*/
-typedef @UINT8_T@ icUInt8Number;
-typedef @UINT16_T@ icUInt16Number;
-typedef @UINT32_T@ icUInt32Number;
-typedef @UINT32_T@ icUInt64Number[2];
+typedef unsigned char icUInt8Number;
+typedef unsigned short icUInt16Number;
+typedef unsigned int icUInt32Number;
+typedef unsigned int icUInt64Number[2];
-typedef @INT8_T@ icInt8Number;
-typedef @INT16_T@ icInt16Number;
-typedef @INT32_T@ icInt32Number;
-typedef @INT32_T@ icInt64Number[2];
+typedef char icInt8Number;
+typedef short icInt16Number;
+typedef int icInt32Number;
+typedef int icInt64Number[2];
#else
Only in lcms-1.19.dfsg-patched/include: Makefile
Only in lcms-1.19.dfsg-patched/jpegicc: .deps
Only in lcms-1.19.dfsg-patched/jpegicc: Makefile
Only in lcms-1.19.dfsg-patched/: lcms.pc
Only in lcms-1.19.dfsg-patched/: libtool
Only in lcms-1.19.dfsg-patched/: Makefile
Only in lcms-1.19.dfsg-patched/python: .deps
Only in lcms-1.19.dfsg-patched/python: Makefile
Only in lcms-1.19.dfsg-patched/samples: .deps
diff -urb lcms-1.19.dfsg/samples/icctrans.c lcms-1.19.dfsg-patched/samples/icctrans.c
--- lcms-1.19.dfsg/samples/icctrans.c 2009-10-30 15:57:45.0 +
+++ lcms-1.19.dfsg-patched/samples/icctrans.c 2013-08-04 10:31:36.608445149 +0100
@@ -500,7 +500,7 @@
Prefix[0] = 0;
if (!lTerse)
-sprintf(Prefix, %s=, C);
+snprintf(Prefix, 20, %s=, C);
if (InHexa)
{
@@ -648,7 +648,9 @@
static
void GetLine(char* Buffer)
{
-scanf(%s, Buffer);
+size_t Buffer_size = sizeof(Buffer);
+fgets(Buffer, (Buffer_size - 1), stdin);
+sscanf(%s, Buffer);
if (toupper(Buffer[0]) == 'Q') { // Quit?
Only in lcms-1.19.dfsg-patched/samples: Makefile
Only in lcms-1.19.dfsg-patched/src: .deps
Only in lcms-1.19.dfsg-patched/src: Makefile
Only in lcms-1.19.dfsg-patched/testbed: .deps
Only in lcms-1.19.dfsg-patched/testbed: Makefile
Only in lcms-1.19.dfsg-patched/tifficc: .deps
Only in lcms-1.19.dfsg-patched/tifficc: Makefile
diff -urb lcms-1.19.dfsg/tifficc/tiffdiff.c lcms-1.19.dfsg-patched/tifficc/tiffdiff.c
--- lcms-1.19.dfsg/tifficc/tiffdiff.c 2009-10-30 15:57:46.0 +
+++ lcms-1.19.dfsg-patched/tifficc/tiffdiff.c 2013-08-04 10:25:27.506059564 +0100
@@ -633,7 +633,7 @@
cmsIT8SetSheetType(hIT8, TIFFDIFF);
-sprintf(Buffer, Differences between %s and %s, TiffName1, TiffName2);
+snprintf(Buffer, 256, Differences between %s and %s, TiffName1, TiffName2);
cmsIT8SetComment(hIT8, Buffer);
Bug#718682: liblcms1: Buffer overflows in Little CMS v1.19
Hi Pedro,
thank you for reporting this security issue.
On 2013-08-04 10:35:46, Pedro R wrote:
diff -urb lcms-1.19.dfsg/samples/icctrans.c
lcms-1.19.dfsg-patched/samples/icctrans.c
--- lcms-1.19.dfsg/samples/icctrans.c 2009-10-30 15:57:45.0 +
+++ lcms-1.19.dfsg-patched/samples/icctrans.c 2013-08-04 10:31:36.608445149
+0100
@@ -500,7 +500,7 @@
Prefix[0] = 0;
if (!lTerse)
-sprintf(Prefix, %s=, C);
+snprintf(Prefix, 20, %s=, C);
if (InHexa)
{
@@ -648,7 +648,9 @@
static
void GetLine(char* Buffer)
{
-scanf(%s, Buffer);
+size_t Buffer_size = sizeof(Buffer);
+fgets(Buffer, (Buffer_size - 1), stdin);
+sscanf(%s, Buffer);
This sscanf call is wrong and introduces a format string vulnerability.
sscanf's signature is int sscanf(const char* str, const char* fmt, ...)
where str is used as input and format is the second argument.
Regards
--
Sebastian Ramacher
signature.asc
Description: Digital signature
