I've looked at the patch you've provided and I must say that I believe
that it is utterly broken with regards to the "integer overflow". I
don't think that I've discovered a single integer overflow that's
been prevented. Attached is what was left over after the investigation.
Several conditions have been changed since they were pure crap.
You can't check against zero if the value may be -1, then
malloc with that value *and* access the resulting array.
(Well you can, but you don't fix anything by ensuring
that 0 is not used as factor).
So we have:
htperes.cc Buffer overflows
cplus-dem.cc negative malloc with out-of-bounds array access
(not sure if this can be successfully exploited)
Please correct me if I'm wrong.
Please run a diff agains the interdiff between the stable
package and the "fixed" packages.
Regards,
Joey
--
Long noun chains don't automatically imply security. -- Bruce Schneier
Please always Cc to me when replying to me on the lists.
diff -u ht-0.5.0/debian/changelog ht-0.5.0/debian/changelog
--- ht-0.5.0/debian/changelog
+++ ht-0.5.0/debian/changelog
@@ -1,3 +1,16 @@
+ht (0.5.0-1woody1) stable-security; urgency=high
+
+ * Security fix pulled from upstream CVS and backported (see #308587)
+ + fix an integer overflow in the ELF segment parsing
+ (cplus-dem.c, htanaly.cc, htcoff.cc, htelf.cc, htpeimp.cc; htpef.cc
+ not present)
+ + fix some buffer overflows in the PE parser
+ (htperes.cc)
+ + this is also Gentoo GLSA 200505-08
+ Thanks a lot to Moritz Muehlenhoff for the report!
+
+ -- Florian Ernst <[EMAIL PROTECTED]> Wed, 11 May 2005 20:36:03 +0200
+
ht (0.5.0-1) unstable; urgency=low
* New upstream version.
only in patch2:
unchanged:
--- ht-0.5.0.orig/htperes.cc
+++ ht-0.5.0/htperes.cc
@@ -94,7 +94,6 @@
peresource_file->seek(peresource_dir_ofs+ofs+sizeof dir+i*8);
peresource_file->read(&entry, sizeof entry);
create_host_struct(&entry, PE_RESOURCE_DIRECTORY_ENTRY_struct,
little_endian);
- char *rm = peresource_string;
if (entry.offset_to_directory & 0x80000000) {
bool hasname = entry.name & 0x80000000;
PE_RESOURCE_DIRECTORY subdir;
@@ -104,24 +103,26 @@
if (hasname) {
peresource_file->seek(peresource_dir_ofs+entry.name & 0x7fffffff);
char *name = getstrw(peresource_file);
- rm += sprintf(rm, "%s [%d]", name,
subdir.name_count+subdir.id_count);
+ snprintf(peresource_string, sizeof
peresource_string, "%s [%d]", name, subdir.name_count+subdir.id_count);
delete name;
} else {
char *s = (!level) ? s =
matchhash((short)entry.name, restypes) : 0;
if (s) {
- rm += sprintf(rm, "ID %04x, %s [%d]",
(short)entry.name, s, subdir.name_count+subdir.id_count);
+ snprintf(peresource_string, sizeof
peresource_string, "ID %04x, %s [%d]", (short)entry.name, s,
subdir.name_count+subdir.id_count);
} else {
- rm += sprintf(rm, "ID %04x [%d]",
(short)entry.name, subdir.name_count+subdir.id_count);
+ snprintf(peresource_string, sizeof
peresource_string, "ID %04x [%d]", (short)entry.name,
subdir.name_count+subdir.id_count);
}
}
void *n = peresource_tree->add_child(node,
peresource_string);
read_resource_dir(n, entry.offset_to_directory &
0x7fffffff, level+1);
} else {
+ char *rm = peresource_string;
+ char *rm_end = rm + sizeof peresource_string;
char *s = matchhash((char)entry.name, languages);
if (s) {
- rm += sprintf(rm, "resource, %s (%04x) ", s,
(short)entry.name);
+ rm += snprintf(rm, rm_end-rm, "resource, %s
(%04x) ", s, (short)entry.name);
} else {
- rm += sprintf(rm, "resource, unknown language
(%04x) ", (short)entry.name);
+ rm += snprintf(rm, rm_end-rm, "resource,
unknown language (%04x) ", (short)entry.name);
}
PE_RESOURCE_DATA_ENTRY data;
@@ -135,11 +136,11 @@
xdata = new ht_pe_resource_leaf();
xdata->offset = dofs;
xdata->size = data.size;
- rm += sprintf(rm, "offset %08x", dofs);
+ rm += snprintf(rm, rm_end-rm, "offset %08x",
dofs);
} else {
- rm += sprintf(rm, "offset ? (rva %08x,
currupt)", data.offset_to_data);
+ rm += snprintf(rm, rm_end-rm, "offset ? (rva
%08x, currupt)", data.offset_to_data);
}
- sprintf(rm, " size %08x", data.size);
+ snprintf(rm, rm_end-rm, " size %08x", data.size);
peresource_tree->add_child(node, peresource_string,
xdata);
}
}
only in patch2:
unchanged:
--- ht-0.5.0.orig/cplus-dem.c
+++ ht-0.5.0/cplus-dem.c
@@ -1690,7 +1690,7 @@
{
return (0);
}
- if (!is_type)
+ if (!is_type && r > 0)
{
/* Create an array for saving the template argument values. */
work->tmpl_argvec = (char**) xmalloc (r * sizeof (char *));
@@ -1718,9 +1718,11 @@
{
/* Save the template argument. */
int len = temp.p - temp.b;
- work->tmpl_argvec[i] = xmalloc (len + 1);
- memcpy (work->tmpl_argvec[i], temp.b, len);
- work->tmpl_argvec[i][len] = '\0';
+ if (len > 0) {
+ work->tmpl_argvec[i] = xmalloc (len + 1);
+ memcpy (work->tmpl_argvec[i], temp.b, len);
+ work->tmpl_argvec[i][len] = '\0';
+ }
}
}
string_delete(&temp);
@@ -1746,9 +1748,12 @@
{
/* Save the template argument. */
int len = r2;
- work->tmpl_argvec[i] = xmalloc (len + 1);
- memcpy (work->tmpl_argvec[i], *mangled, len);
- work->tmpl_argvec[i][len] = '\0';
+ if (len > 0)
+ {
+ work->tmpl_argvec[i] = xmalloc (len + 1);
+ memcpy (work->tmpl_argvec[i], *mangled, len);
+ work->tmpl_argvec[i][len] = '\0';
+ }
}
*mangled += r2;
}
@@ -1792,9 +1797,11 @@
if (!is_type)
{
int len = s->p - s->b;
- work->tmpl_argvec[i] = xmalloc (len + 1);
- memcpy (work->tmpl_argvec[i], s->b, len);
- work->tmpl_argvec[i][len] = '\0';
+ if (len > 0) {
+ work->tmpl_argvec[i] = xmalloc (len + 1);
+ memcpy (work->tmpl_argvec[i], s->b, len);
+ work->tmpl_argvec[i][len] = '\0';
+ }
string_appends (tname, s);
string_delete (s);
@@ -2594,6 +2601,7 @@
char * recurse = (char *)NULL;
char * recurse_dem = (char *)NULL;
+ if (namelength <= 0) return; /* not sure about this one */
recurse = (char *) xmalloc (namelength + 1);
memcpy (recurse, *mangled, namelength);
recurse[namelength] = '\000';
@@ -3730,6 +3738,7 @@
sizeof (char *) * work -> typevec_size);
}
}
+ if (len<=0) return;
tem = xmalloc (len + 1);
memcpy (tem, start, len);
tem[len] = '\0';
@@ -3762,6 +3771,7 @@
sizeof (char *) * work -> ksize);
}
}
+ if (len<=0) return;
tem = xmalloc (len + 1);
memcpy (tem, start, len);
tem[len] = '\0';
@@ -3809,6 +3819,7 @@
{
char *tem;
+ if (len<=0) return;
tem = xmalloc (len + 1);
memcpy (tem, start, len);
tem[len] = '\0';
