Differentiate between non-existent and invalid region tag
authorPanu Matilainen <pmatilai@redhat.com>
Wed, 18 Jan 2012 08:56:35 +0000 (10:56 +0200)
committerPanu Matilainen <pmatilai@redhat.com>
Tue, 3 Apr 2012 12:46:42 +0000 (15:46 +0300)
- Non-existent region tag is very different from existing but invalid
  one - the former is not an error but the latter one is, and needs
  to be handled as such. Previously an invalid region tag would cause
  us to treat it like rpm v3 package on entry, skipping all the region
  sanity checks and then crashing and burning later on when the immutable
  tag is fetched.
- Refer to REGION_TAG_TYPE instead of RPM_BIN_TYPE wrt the expected
  type of region tag for consistency and clarity, they are the same
  exact thing though.
- Should unify these damn copy-slop check one of these days, sigh...
  For now, settling for the easily backportable approach.
- Fixes the other half of CVE-2012-0060

lib/package.c
lib/signature.c

index ae665de..6d0c80d 100644 (file)
@@ -310,14 +310,21 @@ static rpmRC headerVerify(rpmKeyring keyring, rpmVSFlags vsflags,
     }
 
     /* Is there an immutable header region tag? */
-    if (!(entry.info.tag == RPMTAG_HEADERIMMUTABLE
-       && entry.info.type == RPM_BIN_TYPE
-       && entry.info.count == REGION_TAG_COUNT))
-    {
+    if (!(entry.info.tag == RPMTAG_HEADERIMMUTABLE)) {
        rc = RPMRC_NOTFOUND;
        goto exit;
     }
 
+    /* Is the region tag sane? */
+    if (!(entry.info.type == REGION_TAG_TYPE &&
+         entry.info.count == REGION_TAG_COUNT)) {
+       rasprintf(&buf,
+               _("region tag: BAD, tag %d type %d offset %d count %d\n"),
+               entry.info.tag, entry.info.type,
+               entry.info.offset, entry.info.count);
+       goto exit;
+    }
+
     /* Is the trailer within the data area? */
     if (entry.info.offset + REGION_TAG_COUNT > dl) {
        rasprintf(&buf, 
@@ -334,7 +341,7 @@ static rpmRC headerVerify(rpmKeyring keyring, rpmVSFlags vsflags,
 
     if (headerVerifyInfo(1, dl, &info, &entry.info, 1) != -1 ||
        !(entry.info.tag == RPMTAG_HEADERIMMUTABLE
-       && entry.info.type == RPM_BIN_TYPE
+       && entry.info.type == REGION_TAG_TYPE
        && entry.info.count == REGION_TAG_COUNT))
     {
        rasprintf(&buf, 
index 63e59c0..42c4721 100644 (file)
@@ -134,11 +134,17 @@ rpmRC rpmReadSignature(FD_t fd, Header * sighp, sigType sig_type, char ** msg)
     }
 
     /* Is there an immutable header region tag? */
-    if (entry.info.tag == RPMTAG_HEADERSIGNATURES
-       && entry.info.type == RPM_BIN_TYPE
-       && entry.info.count == REGION_TAG_COUNT)
-    {
-
+    if (entry.info.tag == RPMTAG_HEADERSIGNATURES) {
+       /* Is the region tag sane? */
+       if (!(entry.info.type == REGION_TAG_TYPE &&
+             entry.info.count == REGION_TAG_COUNT)) {
+           rasprintf(&buf,
+               _("region tag: BAD, tag %d type %d offset %d count %d\n"),
+               entry.info.tag, entry.info.type,
+               entry.info.offset, entry.info.count);
+           goto exit;
+       }
+       
        /* Is the trailer within the data area? */
        if (entry.info.offset + REGION_TAG_COUNT > dl) {
            rasprintf(&buf, 
@@ -162,7 +168,7 @@ rpmRC rpmReadSignature(FD_t fd, Header * sighp, sigType sig_type, char ** msg)
        xx = headerVerifyInfo(1, dl, &info, &entry.info, 1);
        if (xx != -1 ||
            !((entry.info.tag == RPMTAG_HEADERSIGNATURES || entry.info.tag == RPMTAG_HEADERIMAGE)
-          && entry.info.type == RPM_BIN_TYPE
+          && entry.info.type == REGION_TAG_TYPE
           && entry.info.count == REGION_TAG_COUNT))
        {
            rasprintf(&buf,