[Reproducible-commits] [dpkg] 20/25: libdpkg, dpkg: Fix out-of-bounds read accesses

Holger Levsen holger at layer-acht.org
Tue May 3 08:43:55 UTC 2016


This is an automated email from the git hooks/post-receive script.

holger pushed a commit to annotated tag 1.16.16
in repository dpkg.

commit 3d75e12c5425579993c5cde848223a1891eb693a
Author: Guillem Jover <guillem at debian.org>
Date:   Sat Nov 29 15:56:15 2014 +0100

    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 <megamansec at 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 <megamansec at 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 <megamansec at 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;

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/reproducible/dpkg.git



More information about the Reproducible-commits mailing list