libdpkg, dpkg: Fix out-of-bounds read accesses

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@gmail.com>

debian/changelog

@@ -5,6 +5,9 @@ dpkg (1.17.23) UNRELEASED; urgency=low
   * Skip tar extractor tests if tar is not GNU tar >= 1.27.
   * Reset the trigger cycle tracking on unsatisfied dependencies during
     trigger processing. Closes: #771730
+  * 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@gmail.com>.
 
   [ Updated programs translations ]
   * Basque (Iñaki Larrañaga Murgoitio). Closes: #771893

lib/dpkg/parse.c

@@ -52,49 +52,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 }
 };
 
@@ -122,8 +122,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;
@@ -131,8 +131,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)++)
@@ -154,7 +154,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);

lib/dpkg/parsedump.h

@@ -120,8 +120,11 @@ void
 varbuf_add_arbfield(struct varbuf *vb, const struct arbitraryfield *arbfield,
                     enum fwriteflags flags);
 
+#define FIELD(name) name, sizeof(name) - 1
+
 struct fieldinfo {
   const char *name;
+  size_t namelen;
   freadfunction *rcall;
   fwritefunction *wcall;
   size_t integer;
@@ -142,9 +145,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[];

lib/dpkg/pkg-format.c

@@ -327,14 +327,14 @@ 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 },
-	{ "db:Status-Want", NULL, virt_status_want },
-	{ "db:Status-Status", NULL, virt_status_status },
-	{ "db:Status-Eflag", NULL, virt_status_eflag },
-	{ "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("db:Status-Want"), NULL, virt_status_want },
+	{ FIELD("db:Status-Status"), NULL, virt_status_status },
+	{ FIELD("db:Status-Eflag"), NULL, virt_status_eflag },
+	{ FIELD("source:Package"), NULL, virt_source_package },
+	{ FIELD("source:Version"), NULL, virt_source_version },
 	{ NULL },
 };
 

lib/dpkg/triglib.c

@@ -337,9 +337,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);

src/help.c

@@ -227,7 +227,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;
@@ -287,6 +287,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;