Hi!
On Sat, 2015-04-04 at 08:58:01 +0100, Adam D. Barratt wrote:
> Control: tags -1 -moreinfo +confirmed
> As far as I can see, the fixes all look okay to me (and assuming they've
> been tested on a wheezy system).
Thanks. Although, sorry, I've realized I had forgotten about two other
fixes. Are the attached patches fine to include too? They have been in
unstable/jessie for a while (and approved for jessie while frozen).
Note that the second patch fixes the first one too. Trying to fix the
first problem requires pulling in most of the second patch, and I didn't
want to merge them into a single commit, to keep them as independent
fixes.
Thanks,
Guillem
From 07434a794527d37f1bec62aee3b69bd4cb671d6f Mon Sep 17 00:00:00 2001
From: Guillem Jover <guil...@debian.org>
Date: Tue, 11 Nov 2014 17:37:04 +0100
Subject: [PATCH 1/2] libdpkg: Do not match partial field names in control
files
Cherry picked from commit 611305ef0e85092cc24887e040c19e9e808dd633.
There is currently no instance of any misspelled field names known to
dpkg in Debian. Only known field names are possibly affected.
Regression introduced in commit 864e230e90de1cef94c81f10582e6d99717d593b.
Closes: #769119
---
debian/changelog | 2 ++
lib/dpkg/parse.c | 6 ++++--
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/debian/changelog b/debian/changelog
index 9c29d6f..d7751ab 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -29,6 +29,8 @@ dpkg (1.16.15+nmu1) UNRELEASED; urgency=low
and they come from the package fields, which are under user control.
Regression introduced in dpkg 1.16.0. Fixes CVE-2014-8625. Closes: #768485
Reported by Joshua Rogers <megaman...@gmail.com>.
+ * Do not match partial field names in control files. Closes: #769119
+ Regression introduced in dpkg 1.10.
[ Updated scripts translations ]
* Fix typos in German (Helge Kreutzmann)
diff --git a/lib/dpkg/parse.c b/lib/dpkg/parse.c
index b51ca1b..446805b 100644
--- a/lib/dpkg/parse.c
+++ b/lib/dpkg/parse.c
@@ -130,7 +130,8 @@ pkg_parse_field(struct parsedb_state *ps, struct field_state *fs,
}
for (fip = fieldinfos, ip = fs->fieldencountered; fip->name; fip++, ip++)
- if (strncasecmp(fip->name, fs->fieldstart, fs->fieldlen) == 0)
+ if (strncasecmp(fip->name, fs->fieldstart, fs->fieldlen) == 0 &&
+ fip->name[fs->fieldlen] == '\0')
break;
if (fip->name) {
if ((*ip)++)
@@ -151,7 +152,8 @@ pkg_parse_field(struct parsedb_state *ps, struct field_state *fs,
fs->fieldlen, fs->fieldstart);
larpp = &pkg_obj->pkgbin->arbs;
while ((arp = *larpp) != NULL) {
- if (strncasecmp(arp->name, fs->fieldstart, fs->fieldlen) == 0)
+ if (strncasecmp(arp->name, fs->fieldstart, fs->fieldlen) == 0 &&
+ arp->name[fs->fieldlen] == '\0')
parse_error(ps,
_("duplicate value for user-defined field `%.*s'"),
fs->fieldlen, fs->fieldstart);
--
2.2.1.209.g41e5f3a
From ece3ccdf17da15989c2c9f031c09cce114bce666 Mon Sep 17 00:00:00 2001
From: Guillem Jover <guil...@debian.org>
Date: Sat, 29 Nov 2014 15:56:15 +0100
Subject: [PATCH 2/2] libdpkg, dpkg: Fix out-of-bounds read accesses
Cherry picked from commit fa1cfce24dc7c0659cb16b4a6ff09f660e318731.
Limit the buffer accesses to the size of the buffer being accessed. This
affects reads done when parsing field and trigger names, or checking the
package ownership of conffiles and directories.
Use a new length member for struct fieldinfo and nickname to avoid
recomputing the same known length over and over again, but use strlen()
instead for arbitrary fields, conffiles and directories to avoid
increaseing the memory footprint too much.
Reported-by: Joshua Rogers <megaman...@gmail.com>
---
debian/changelog | 3 ++
lib/dpkg/parse.c | 84 +++++++++++++++++++++++++--------------------------
lib/dpkg/parsedump.h | 6 ++++
lib/dpkg/pkg-format.c | 10 +++---
lib/dpkg/triglib.c | 4 +--
src/help.c | 3 +-
6 files changed, 60 insertions(+), 50 deletions(-)
diff --git a/debian/changelog b/debian/changelog
index d7751ab..0c94fdd 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -31,6 +31,9 @@ dpkg (1.16.15+nmu1) UNRELEASED; urgency=low
Reported by Joshua Rogers <megaman...@gmail.com>.
* Do not match partial field names in control files. Closes: #769119
Regression introduced in dpkg 1.10.
+ * Fix out-of-bounds buffer read accesses when parsing field and trigger
+ names or checking package ownership of conffiles and directories.
+ Reported by Joshua Rogers <megaman...@gmail.com>.
[ Updated scripts translations ]
* Fix typos in German (Helge Kreutzmann)
diff --git a/lib/dpkg/parse.c b/lib/dpkg/parse.c
index 446805b..e790ec5 100644
--- a/lib/dpkg/parse.c
+++ b/lib/dpkg/parse.c
@@ -51,49 +51,49 @@
*/
const struct fieldinfo fieldinfos[]= {
/* Note: Capitalization of field name strings is important. */
- { "Package", f_name, w_name },
- { "Essential", f_boolean, w_booleandefno, PKGIFPOFF(essential) },
- { "Status", f_status, w_status },
- { "Priority", f_priority, w_priority },
- { "Section", f_section, w_section },
- { "Installed-Size", f_charfield, w_charfield, PKGIFPOFF(installedsize) },
- { "Origin", f_charfield, w_charfield, PKGIFPOFF(origin) },
- { "Maintainer", f_charfield, w_charfield, PKGIFPOFF(maintainer) },
- { "Bugs", f_charfield, w_charfield, PKGIFPOFF(bugs) },
- { "Architecture", f_architecture, w_architecture },
- { "Multi-Arch", f_multiarch, w_multiarch, PKGIFPOFF(multiarch) },
- { "Source", f_charfield, w_charfield, PKGIFPOFF(source) },
- { "Version", f_version, w_version, PKGIFPOFF(version) },
- { "Revision", f_revision, w_null },
- { "Config-Version", f_configversion, w_configversion },
- { "Replaces", f_dependency, w_dependency, dep_replaces },
- { "Provides", f_dependency, w_dependency, dep_provides },
- { "Depends", f_dependency, w_dependency, dep_depends },
- { "Pre-Depends", f_dependency, w_dependency, dep_predepends },
- { "Recommends", f_dependency, w_dependency, dep_recommends },
- { "Suggests", f_dependency, w_dependency, dep_suggests },
- { "Breaks", f_dependency, w_dependency, dep_breaks },
- { "Conflicts", f_dependency, w_dependency, dep_conflicts },
- { "Enhances", f_dependency, w_dependency, dep_enhances },
- { "Conffiles", f_conffiles, w_conffiles },
- { "Filename", f_filecharf, w_filecharf, FILEFOFF(name) },
- { "Size", f_filecharf, w_filecharf, FILEFOFF(size) },
- { "MD5sum", f_filecharf, w_filecharf, FILEFOFF(md5sum) },
- { "MSDOS-Filename", f_filecharf, w_filecharf, FILEFOFF(msdosname) },
- { "Description", f_charfield, w_charfield, PKGIFPOFF(description) },
- { "Triggers-Pending", f_trigpend, w_trigpend },
- { "Triggers-Awaited", f_trigaw, w_trigaw },
+ { FIELD("Package"), f_name, w_name },
+ { FIELD("Essential"), f_boolean, w_booleandefno, PKGIFPOFF(essential) },
+ { FIELD("Status"), f_status, w_status },
+ { FIELD("Priority"), f_priority, w_priority },
+ { FIELD("Section"), f_section, w_section },
+ { FIELD("Installed-Size"), f_charfield, w_charfield, PKGIFPOFF(installedsize) },
+ { FIELD("Origin"), f_charfield, w_charfield, PKGIFPOFF(origin) },
+ { FIELD("Maintainer"), f_charfield, w_charfield, PKGIFPOFF(maintainer) },
+ { FIELD("Bugs"), f_charfield, w_charfield, PKGIFPOFF(bugs) },
+ { FIELD("Architecture"), f_architecture, w_architecture },
+ { FIELD("Multi-Arch"), f_multiarch, w_multiarch, PKGIFPOFF(multiarch) },
+ { FIELD("Source"), f_charfield, w_charfield, PKGIFPOFF(source) },
+ { FIELD("Version"), f_version, w_version, PKGIFPOFF(version) },
+ { FIELD("Revision"), f_revision, w_null },
+ { FIELD("Config-Version"), f_configversion, w_configversion },
+ { FIELD("Replaces"), f_dependency, w_dependency, dep_replaces },
+ { FIELD("Provides"), f_dependency, w_dependency, dep_provides },
+ { FIELD("Depends"), f_dependency, w_dependency, dep_depends },
+ { FIELD("Pre-Depends"), f_dependency, w_dependency, dep_predepends },
+ { FIELD("Recommends"), f_dependency, w_dependency, dep_recommends },
+ { FIELD("Suggests"), f_dependency, w_dependency, dep_suggests },
+ { FIELD("Breaks"), f_dependency, w_dependency, dep_breaks },
+ { FIELD("Conflicts"), f_dependency, w_dependency, dep_conflicts },
+ { FIELD("Enhances"), f_dependency, w_dependency, dep_enhances },
+ { FIELD("Conffiles"), f_conffiles, w_conffiles },
+ { FIELD("Filename"), f_filecharf, w_filecharf, FILEFOFF(name) },
+ { FIELD("Size"), f_filecharf, w_filecharf, FILEFOFF(size) },
+ { FIELD("MD5sum"), f_filecharf, w_filecharf, FILEFOFF(md5sum) },
+ { FIELD("MSDOS-Filename"), f_filecharf, w_filecharf, FILEFOFF(msdosname) },
+ { FIELD("Description"), f_charfield, w_charfield, PKGIFPOFF(description) },
+ { FIELD("Triggers-Pending"), f_trigpend, w_trigpend },
+ { FIELD("Triggers-Awaited"), f_trigaw, w_trigaw },
/* Note that aliases are added to the nicknames table. */
{ NULL }
};
static const struct nickname nicknames[] = {
/* Note: Capitalization of these strings is important. */
- { .nick = "Recommended", .canon = "Recommends" },
- { .nick = "Optional", .canon = "Suggests" },
- { .nick = "Class", .canon = "Priority" },
- { .nick = "Package-Revision", .canon = "Revision" },
- { .nick = "Package_Revision", .canon = "Revision" },
+ { NICK("Recommended"), .canon = "Recommends" },
+ { NICK("Optional"), .canon = "Suggests" },
+ { NICK("Class"), .canon = "Priority" },
+ { NICK("Package-Revision"), .canon = "Revision" },
+ { NICK("Package_Revision"), .canon = "Revision" },
{ .nick = NULL }
};
@@ -121,8 +121,8 @@ pkg_parse_field(struct parsedb_state *ps, struct field_state *fs,
int *ip;
for (nick = nicknames; nick->nick; nick++)
- if (strncasecmp(nick->nick, fs->fieldstart, fs->fieldlen) == 0 &&
- nick->nick[fs->fieldlen] == '\0')
+ if (nick->nicklen == (size_t)fs->fieldlen &&
+ strncasecmp(nick->nick, fs->fieldstart, fs->fieldlen) == 0)
break;
if (nick->nick) {
fs->fieldstart = nick->canon;
@@ -130,8 +130,8 @@ pkg_parse_field(struct parsedb_state *ps, struct field_state *fs,
}
for (fip = fieldinfos, ip = fs->fieldencountered; fip->name; fip++, ip++)
- if (strncasecmp(fip->name, fs->fieldstart, fs->fieldlen) == 0 &&
- fip->name[fs->fieldlen] == '\0')
+ if (fip->namelen == (size_t)fs->fieldlen &&
+ strncasecmp(fip->name, fs->fieldstart, fs->fieldlen) == 0)
break;
if (fip->name) {
if ((*ip)++)
@@ -153,7 +153,7 @@ pkg_parse_field(struct parsedb_state *ps, struct field_state *fs,
larpp = &pkg_obj->pkgbin->arbs;
while ((arp = *larpp) != NULL) {
if (strncasecmp(arp->name, fs->fieldstart, fs->fieldlen) == 0 &&
- arp->name[fs->fieldlen] == '\0')
+ strlen(arp->name) == (size_t)fs->fieldlen)
parse_error(ps,
_("duplicate value for user-defined field `%.*s'"),
fs->fieldlen, fs->fieldstart);
diff --git a/lib/dpkg/parsedump.h b/lib/dpkg/parsedump.h
index f39071e..57a978b 100644
--- a/lib/dpkg/parsedump.h
+++ b/lib/dpkg/parsedump.h
@@ -107,8 +107,11 @@ fwritefunction w_architecture;
fwritefunction w_filecharf;
fwritefunction w_trigpend, w_trigaw;
+#define FIELD(name) name, sizeof(name) - 1
+
struct fieldinfo {
const char *name;
+ size_t namelen;
freadfunction *rcall;
fwritefunction *wcall;
size_t integer;
@@ -129,9 +132,12 @@ void parse_ensure_have_field(struct parsedb_state *ps,
#define MSDOS_EOF_CHAR '\032' /* ^Z */
+#define NICK(name) .nick = name, .nicklen = sizeof(name) - 1
+
struct nickname {
const char *nick;
const char *canon;
+ size_t nicklen;
};
extern const struct fieldinfo fieldinfos[];
diff --git a/lib/dpkg/pkg-format.c b/lib/dpkg/pkg-format.c
index 74a8599..4cb9984 100644
--- a/lib/dpkg/pkg-format.c
+++ b/lib/dpkg/pkg-format.c
@@ -294,11 +294,11 @@ virt_source_version(struct varbuf *vb,
}
const struct fieldinfo virtinfos[] = {
- { "binary:Package", NULL, virt_package },
- { "binary:Summary", NULL, virt_summary },
- { "db:Status-Abbrev", NULL, virt_status_abbrev },
- { "source:Package", NULL, virt_source_package },
- { "source:Version", NULL, virt_source_version },
+ { FIELD("binary:Package"), NULL, virt_package },
+ { FIELD("binary:Summary"), NULL, virt_summary },
+ { FIELD("db:Status-Abbrev"), NULL, virt_status_abbrev },
+ { FIELD("source:Package"), NULL, virt_source_package },
+ { FIELD("source:Version"), NULL, virt_source_version },
{ NULL },
};
diff --git a/lib/dpkg/triglib.c b/lib/dpkg/triglib.c
index 0a46a81..3233276 100644
--- a/lib/dpkg/triglib.c
+++ b/lib/dpkg/triglib.c
@@ -333,9 +333,9 @@ trk_explicit_interest_change(const char *trig, struct pkginfo *pkg,
while (trk_explicit_f && trk_explicit_fgets(buf, sizeof(buf)) >= 0) {
const char *pkgname = pkgbin_name(pkg, pkgbin, pnaw_nonambig);
- int len = strlen(pkgname);
+ size_t len = strlen(pkgname);
- if (strncmp(buf, pkgname, len) == 0 &&
+ if (strncmp(buf, pkgname, len) == 0 && len < sizeof(buf) &&
(buf[len] == '\0' || buf[len] == '/'))
continue;
fprintf(file->fp, "%s\n", buf);
diff --git a/src/help.c b/src/help.c
index bb9350e..6905ee7 100644
--- a/src/help.c
+++ b/src/help.c
@@ -221,7 +221,7 @@ dir_has_conffiles(struct filenamenode *file, struct pkginfo *pkg)
if (conff->obsolete)
continue;
if (strncmp(file->name, conff->name, namelen) == 0 &&
- conff->name[namelen] == '/') {
+ strlen(conff->name) > namelen && conff->name[namelen] == '/') {
debug(dbg_veryverbose, "directory %s has conffile %s from %s",
file->name, conff->name, pkg_name(pkg, pnaw_always));
return true;
@@ -281,6 +281,7 @@ dir_is_used_by_pkg(struct filenamenode *file, struct pkginfo *pkg,
node->namenode->name);
if (strncmp(file->name, node->namenode->name, namelen) == 0 &&
+ strlen(node->namenode->name) > namelen &&
node->namenode->name[namelen] == '/') {
debug(dbg_veryverbose, "dir_is_used_by_pkg yes");
return true;
--
2.2.1.209.g41e5f3a
