[prev in list] [next in list] [prev in thread] [next in thread]
List: oss-security
Subject: [oss-security] CVE request: lcms (old issues)
From: Tomas Hoger <thoger () redhat ! com>
Date: 2008-11-28 16:12:40
Message-ID: 20081128171240.02a1c350 () redhat ! com
[Download RAW message or body]
Hi!
While digging around CVE-2007-2741, I found out that there are 2 other
issues that were quite silently fixed in the Little CMS updates tagged
as fixing CVE-2007-2741 as done by various vendors.
The issues are:
The ReadEmbeddedTextTag in src/cmsio1.c did not properly check amount
of data read from the input file to the buffer provided as one of it's
arguments. Value read from the file was used as an upper bound without
any validation.
This issue was fixed upstream in 1.16. Attached is the patch against
1.15 lcms packages as was used in SuSE security updates (original name
of the patch as used in SuSE and Mandriva SRPMS is
lcms-CVE-2007-2741.patch, but it is not a fix for CVE-2007-2741,
CVE-2007-2741 was fixed upstream in 1.15 and the correct patch for it
is named named liblcms-<version>-icc.diff in pre-1.15 SuSE / Mandriva
SRPMS).
Upstream CVS commit:
http://lcms.cvs.sourceforge.net/viewvc/lcms/lcms/src/cmsio1.c?r1=1.33&r2=1.34
Another issue is unsigned -> signed integer cast issue in cmsAllocGamma
in src/cmsgamma.c. The argument to this function - nEntries - may be
read from the file and not validated before cmsAllocGamma is called.
As nEntries in cmsAllocGamma is signed integer, it's value may possibly
be negative and can result in an insufficient memory allocation.
This issue was fixed upstream in 1.17. Again, attached is the patch
extracted from SuSE security updates for 1.15. Original name was
lcms-gamma-overflow.patch.
Upstream CVS commit:
http://lcms.cvs.sourceforge.net/viewvc/lcms/lcms/src/cmsgamma.c?view=diff&r1=1.16&r2=1.17
As both of these fixes date back to 2007, and were used in the security
advisory in 2007, they may need 2007 CVE id. Steven, can you get us
some? Thank you!
--
Tomas Hoger / Red Hat Security Response Team
["lcms-1.15-ReadEmbeddedTextTag-sizechecks.diff" (text/x-patch)]
--- src/cmsio1.c
+++ src/cmsio1.c
@@ -345,13 +345,14 @@
ErrorCleanup:
Icc ->Close(Icc);
- free(Icc);
if (lIsFromMemory)
cmsSignalError(LCMS_ERRC_ABORTED, "Corrupted memory profile");
else
cmsSignalError(LCMS_ERRC_ABORTED, "Corrupted profile: '%s'", Icc->PhysicalFile);
+
+ free(Icc);
return NULL;
}
@@ -1317,7 +1318,7 @@
// handles such special case.
static
-int ReadEmbeddedTextTag(LPLCMSICCPROFILE Icc, size_t size, char* Name)
+int ReadEmbeddedTextTag(LPLCMSICCPROFILE Icc, size_t size, char* Name, size_t size_max)
{
icTagTypeSignature BaseType;
@@ -1336,18 +1337,25 @@
icUInt8Number ScriptCodeCount;
Icc ->Read(&AsciiCount, sizeof(icUInt32Number), 1, Icc);
+
+ if (size < sizeof(icUInt32Number)) return (int) size;
size -= sizeof(icUInt32Number);
AdjustEndianess32((LPBYTE) &AsciiCount);
- Icc ->Read(Name, 1, AsciiCount, Icc);
+ Icc ->Read(Name, 1,
+ (AsciiCount >= size_max) ? (size_max-1) : AsciiCount, Icc);
+
+ if (size < AsciiCount) return (int) size;
size -= AsciiCount;
// Skip Unicode code
Icc ->Read(&UnicodeCode, sizeof(icUInt32Number), 1, Icc);
+ if (size < sizeof(icUInt32Number)) return (int) size;
size -= sizeof(icUInt32Number);
Icc ->Read(&UnicodeCount, sizeof(icUInt32Number), 1, Icc);
+ if (size < sizeof(icUInt32Number)) return (int) size;
size -= sizeof(icUInt32Number);
AdjustEndianess32((LPBYTE) &UnicodeCount);
@@ -1378,8 +1386,21 @@
case icSigCopyrightTag: // Broken profiles from agfa does store copyright info in such type
case icSigTextType:
+ {
+ char Dummy;
+ size_t i, Missing = 0;
+
+ if (size >= size_max) {
+
+ Missing = size - size_max + 1;
+ size = size_max - 1;
+ }
Icc -> Read(Name, 1, size, Icc);
+
+ for (i=0; i < Missing; i++)
+ Icc -> Read(&Dummy, 1, 1, Icc);
+ }
break;
// MultiLocalizedUnicodeType, V4 only
@@ -1453,7 +1474,7 @@
AdjustEndianessArray16((LPWORD) wchar, Len / 2);
wchar[Len / 2] = L'\0';
- i = wcstombs(Name, wchar, 2047 );
+ i = wcstombs(Name, wchar, size_max );
if (i == ((size_t) -1)) {
Name[0] = 0; // Error
@@ -1475,7 +1496,7 @@
// Take an ASCII item
-int LCMSEXPORT cmsReadICCText(cmsHPROFILE hProfile, icTagSignature sig, char *Name)
+int LCMSEXPORT cmsReadICCTextEx(cmsHPROFILE hProfile, icTagSignature sig, char *Name, size_t size_max)
{
LPLCMSICCPROFILE Icc = (LPLCMSICCPROFILE) (LPSTR) hProfile;
size_t offset, size;
@@ -1497,10 +1518,15 @@
if (Icc -> Seek(Icc, offset))
return -1;
- return ReadEmbeddedTextTag(Icc, size, Name);
-
+ return ReadEmbeddedTextTag(Icc, size, Name, size_max);
}
+// Keep compatibility with older versions
+
+int LCMSEXPORT cmsReadICCText(cmsHPROFILE hProfile, icTagSignature sig, char *Text)
+{
+ return cmsReadICCTextEx(hProfile, sig, Text, 512);
+}
// Take an XYZ item
@@ -1796,6 +1822,7 @@
if (!CheckHeader(v->NamedColorList, &nc2)) {
cmsSignalError(LCMS_ERRC_WARNING, "prefix/suffix/device for named color profiles mismatch.");
+ return 0;
}
strncpy(v ->NamedColorList->Prefix, nc2.prefix, 32);
@@ -2030,7 +2057,7 @@
if (cmsIsTag(hProfile, icSigCopyrightTag))
{
- char Copyright[2048];
+ char Copyright[512];
cmsReadICCText(hProfile, icSigCopyrightTag, Copyright);
strcat(Info, Copyright);
@@ -2047,7 +2074,7 @@
if (cmsIsTag(hProfile, K007))
{
- char MonCal[1024];
+ char MonCal[512];
cmsReadICCText(hProfile, K007, MonCal);
strcat(Info, MonCal);
@@ -2094,7 +2121,7 @@
return FALSE;
}
- if (cmsReadICCText(hProfile, icSigCharTargetTag, *Data) < 0)
+ if (cmsReadICCTextEx(hProfile, icSigCharTargetTag, *Data, *len) < 0)
return FALSE;
(*Data)[*len] = 0; // Force a zero marker. Shouldn't be needed, but is
@@ -2200,8 +2227,8 @@
sec ->deviceModel = DescStruct.deviceModel;
sec ->technology = DescStruct.technology;
- if (ReadEmbeddedTextTag(Icc, size, sec ->Manufacturer) < 0) return NULL;
- if (ReadEmbeddedTextTag(Icc, size, sec ->Model) < 0) return NULL;
+ if (ReadEmbeddedTextTag(Icc, size, sec ->Manufacturer, 512) < 0) return NULL;
+ if (ReadEmbeddedTextTag(Icc, size, sec ->Model, 512) < 0) return NULL;
}
@@ -2335,7 +2362,7 @@
return NULL;
}
- ReadEmbeddedTextTag(Icc, 256, gex ->Description);
+ ReadEmbeddedTextTag(Icc, 256, gex ->Description, 512);
// Read viewing conditions
@@ -3566,12 +3593,14 @@
// update BytesSaved so caller knows how many bytes are needed for MemPtr
*BytesNeeded = Icc ->UsedSpace;
+ CopyMemory(Icc, &Keep, sizeof(LCMSICCPROFILE));
return TRUE;
}
if (*BytesNeeded < Icc ->UsedSpace) {
// need at least UsedSpace in MemPtr to continue
+ CopyMemory(Icc, &Keep, sizeof(LCMSICCPROFILE));
return FALSE;
}
["lcms-1.15-cmsAllocGamma-overflow.diff" (text/x-patch)]
--- src/cmsgamma.c
+++ src/cmsgamma.c
@@ -114,9 +114,9 @@
LPGAMMATABLE p;
size_t size;
- if (nEntries > 65530) {
- cmsSignalError(LCMS_ERRC_WARNING, "Couldn't create gammatable of more than \
65530 entries; 65530 assumed");
- nEntries = 65530;
+ if (nEntries > 65530 || nEntries < 0) {
+ cmsSignalError(LCMS_ERRC_ABORTED, "Couldn't create gammatable of more than \
65530 entries"); + return NULL;
}
size = sizeof(GAMMATABLE) + (sizeof(WORD) * (nEntries-1));
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic