- fix: parse header data more carefully.

CVS patchset: 5701
CVS date: 2002/09/04 01:52:26

diff --git a/CHANGES b/CHANGES
index f745206..f339ef1 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -288,6 +288,7 @@
        - fix: resurrect --triggers (#73330).
        - python: typo in NOKEY exception string.
        - fix: parse pgp packets more carefully.
+       - fix: parse header data more carefully.
 
 4.0.3 -> 4.0.4:
        - solaris: translate i86pc to i386 (#57182).

diff --git a/lib/package.c b/lib/package.c
index e28bf2b..99bb59e 100644 (file)
--- a/lib/package.c
+++ b/lib/package.c
@@ -96,7 +96,7 @@ static int typeAlign[16] =  {
  */
 #define hdrchkRange(_dl, _off)         ((_off) < 0 || (_off) > (_dl))
 
-void headerMergeLegacySigs(Header h, const Header sig)
+void headerMergeLegacySigs(Header h, const Header sigh)
 {
     HFD_t hfd = (HFD_t) headerFreeData;
     HAE_t hae = (HAE_t) headerAddEntry;
@@ -105,7 +105,7 @@ void headerMergeLegacySigs(Header h, const Header sig)
     const void * ptr;
     int xx;
 
-    for (hi = headerInitIterator(sig);
+    for (hi = headerInitIterator(sigh);
         headerNextIterator(hi, &tag, &type, &ptr, &count);
         ptr = hfd(ptr, type))
     {
@@ -144,8 +144,34 @@ void headerMergeLegacySigs(Header h, const Header sig)
            /*@switchbreak@*/ break;
        }
        if (ptr == NULL) continue;      /* XXX can't happen */
-       if (!headerIsEntry(h, tag))
-           xx = hae(h, tag, type, ptr, count);
+       if (!headerIsEntry(h, tag)) {
+           if (hdrchkType(type))
+               continue;
+           if (count < 0 || hdrchkData(count))
+               continue;
+           switch(type) {
+           case RPM_NULL_TYPE:
+               continue;
+               /*@notreached@*/ /*@switchbreak@*/ break;
+           case RPM_CHAR_TYPE:
+           case RPM_INT8_TYPE:
+           case RPM_INT16_TYPE:
+           case RPM_INT32_TYPE:
+               if (count != 1)
+                   continue;
+               /*@switchbreak@*/ break;
+           case RPM_STRING_TYPE:
+           case RPM_BIN_TYPE:
+               if (count >= 16*1024)
+                   continue;
+               /*@switchbreak@*/ break;
+           case RPM_STRING_ARRAY_TYPE:
+           case RPM_I18NSTRING_TYPE:
+               continue;
+               /*@notreached@*/ /*@switchbreak@*/ break;
+           }
+           xx = hae(h, tag, type, ptr, count);
+       }
     }
     hi = headerFreeIterator(hi);
 }
@@ -153,7 +179,7 @@ void headerMergeLegacySigs(Header h, const Header sig)
 Header headerRegenSigHeader(const Header h, int noArchiveSize)
 {
     HFD_t hfd = (HFD_t) headerFreeData;
-    Header sig = rpmNewSignature();
+    Header sigh = rpmNewSignature();
     HeaderIterator hi;
     int_32 tag, stag, type, count;
     const void * ptr;
@@ -202,11 +228,11 @@ Header headerRegenSigHeader(const Header h, int noArchiveSize)
            /*@switchbreak@*/ break;
        }
        if (ptr == NULL) continue;      /* XXX can't happen */
-       if (!headerIsEntry(sig, stag))
-           xx = headerAddEntry(sig, stag, type, ptr, count);
+       if (!headerIsEntry(sigh, stag))
+           xx = headerAddEntry(sigh, stag, type, ptr, count);
     }
     hi = headerFreeIterator(hi);
-    return sig;
+    return sigh;
 }
 
 /**
@@ -308,7 +334,7 @@ rpmRC headerCheck(rpmts ts, const void * uh, size_t uc, const char ** msg)
 /*@=boundsread@*/
     int_32 ildl[2];
     int_32 pvlen = sizeof(ildl) + (il * sizeof(*pe)) + dl;
-    unsigned char * dataStart = (char *) (pe + il);
+    unsigned char * dataStart = (unsigned char *) (pe + il);
     indexEntry entry = memset(alloca(sizeof(*entry)), 0, sizeof(*entry));
     entryInfo info = memset(alloca(sizeof(*info)), 0, sizeof(*info));
     const void * sig = NULL;
@@ -477,7 +503,7 @@ verifyinfo_exit:
        /* XXX only V3 signatures for now. */
        if (dig->signature.version != 3) {
            rpmMessage(RPMMESS_WARNING,
-               _("only V3 signatures can be verified, skipping V%u signature"),
+               _("only V3 signatures can be verified, skipping V%u signature\n"),
                dig->signature.version);
            rpmtsCleanDig(ts);
            goto verifyinfo_exit;
@@ -517,7 +543,7 @@ verifyinfo_exit:
        /* XXX only V3 signatures for now. */
        if (dig->signature.version != 3) {
            rpmMessage(RPMMESS_WARNING,
-               _("only V3 signatures can be verified, skipping V%u signature"),
+               _("only V3 signatures can be verified, skipping V%u signature\n"),
                dig->signature.version);
            rpmtsCleanDig(ts);
            goto verifyinfo_exit;
@@ -617,7 +643,7 @@ rpmRC rpmReadHeader(rpmts ts, FD_t fd, Header *hdrp, const char ** msg)
     ei = NULL; /* XXX will be freed with header */
     
 exit:
-    if (rc == RPMRC_OK && hdrp)
+    if (rc == RPMRC_OK && hdrp && h)
        *hdrp = headerLink(h);
     ei = _free(ei);
     h = headerFree(h);
@@ -733,7 +759,8 @@ int rpmReadPackageFile(rpmts ts, FD_t fd, const char * fn, Header * hdrp)
     msg = NULL;
     rc = rpmReadHeader(ts, fd, &h, &msg);
     if (rc != RPMRC_OK || h == NULL) {
-       rpmError(RPMERR_FREAD, _("%s: headerRead failed: %s\n"), fn, msg);
+       rpmError(RPMERR_FREAD, _("%s: headerRead failed: %s"), fn,
+               (msg && *msg ? msg : "\n"));
        msg = _free(msg);
        goto exit;
     }
@@ -768,7 +795,7 @@ int rpmReadPackageFile(rpmts ts, FD_t fd, const char * fn, Header * hdrp)
        /* XXX only V3 signatures for now. */
        if (dig->signature.version != 3) {
            rpmMessage(RPMMESS_WARNING,
-               _("only V3 signatures can be verified, skipping V%u signature"),
+               _("only V3 signatures can be verified, skipping V%u signature\n"),
                dig->signature.version);
            rc = RPMRC_OK;
            goto exit;
@@ -792,7 +819,7 @@ int rpmReadPackageFile(rpmts ts, FD_t fd, const char * fn, Header * hdrp)
        /* XXX only V3 signatures for now. */
        if (dig->signature.version != 3) {
            rpmMessage(RPMMESS_WARNING,
-               _("only V3 signatures can be verified, skipping V%u signature"),
+               _("only V3 signatures can be verified, skipping V%u signature\n"),
                dig->signature.version);
            rc = RPMRC_OK;
            goto exit;
@@ -822,7 +849,7 @@ int rpmReadPackageFile(rpmts ts, FD_t fd, const char * fn, Header * hdrp)
        /* XXX only V3 signatures for now. */
        if (dig->signature.version != 3) {
            rpmMessage(RPMMESS_WARNING,
-               _("only V3 signatures can be verified, skipping V%u signature"),
+               _("only V3 signatures can be verified, skipping V%u signature\n"),
                dig->signature.version);
            rc = RPMRC_OK;
            goto exit;

diff --git a/lib/rpmchecksig.c b/lib/rpmchecksig.c
index e4615df..37c5667 100644 (file)
--- a/lib/rpmchecksig.c
+++ b/lib/rpmchecksig.c
@@ -793,7 +793,7 @@ int rpmVerifySignatures(QVA_t qva, rpmts ts, FD_t fd,
                /* XXX only V3 signatures for now. */
                if (sigp->version != 3) {
                    rpmError(RPMERR_SIGVFY,
-               _("only V3 signatures can be verified, skipping V%u signature"),
+               _("only V3 signatures can be verified, skipping V%u signature\n"),
                        sigp->version);
                    continue;
                }

diff --git a/rpmdb/header.c b/rpmdb/header.c
index ff48007..9c2ce1b 100644 (file)
--- a/rpmdb/header.c
+++ b/rpmdb/header.c
@@ -334,72 +334,68 @@ unsigned int headerSizeof(/*@null@*/ Header h, enum hMagic magicp)
 
 /**
  * Return length of entry data.
- * @todo Remove sanity check exit's.
  * @param type         entry data type
  * @param p            entry data
  * @param count                entry item count
  * @param onDisk       data is concatenated strings (with NUL's))?
- * @return             no. bytes in data
+ * @param pend         pointer to end of data (or NULL)
+ * @return             no. bytes in data, -1 on failure
  */
 /*@mayexit@*/
-static int dataLength(int_32 type, hPTR_t p, int_32 count, int onDisk)
+static int dataLength(int_32 type, hPTR_t p, int_32 count, int onDisk,
+               hPTR_t pend)
        /*@*/
 {
+    const unsigned char * s = p;
+    const unsigned char * se = pend;
     int length = 0;
 
     switch (type) {
     case RPM_STRING_TYPE:
-       if (count == 1) {       /* Special case -- p is just the string */
-           length = strlen(p) + 1;
-           break;
+       if (count != 1)
+           return -1;
+       while (*s++) {
+           if (se && s > se)
+               return -1;
+           length++;
        }
-        /* This should not be allowed */
-       /*@-modfilesys@*/
-       fprintf(stderr, _("dataLength() RPM_STRING_TYPE count must be 1.\n"));
-       /*@=modfilesys@*/
-       exit(EXIT_FAILURE);
-       /*@notreached@*/ break;
+       length++;       /* count nul terminator too. */
+       break;
 
     case RPM_STRING_ARRAY_TYPE:
     case RPM_I18NSTRING_TYPE:
-    {  int i;
-
-       /* This is like RPM_STRING_TYPE, except it's *always* an array */
-       /* Compute sum of length of all strings, including null terminators */
-       i = count;
+       /* These are like RPM_STRING_TYPE, except they're *always* an array */
+       /* Compute sum of length of all strings, including nul terminators */
 
        if (onDisk) {
-           const char * chptr = p;
-           int thisLen;
-
-           while (i--) {
-               thisLen = strlen(chptr) + 1;
-               length += thisLen;
-               chptr += thisLen;
+           while (count--) {
+               length++;       /* count nul terminator too */
+               while (*s++) {
+                   if (se && s > se)
+                       return -1;
+                   length++;
+               }
            }
        } else {
-           const char ** src = (const char **)p;
+           const char ** av = (const char **)p;
 /*@-boundsread@*/
-           while (i--) {
+           while (count--) {
                /* add one for null termination */
-               length += strlen(*src++) + 1;
+               length += strlen(*av++) + 1;
            }
 /*@=boundsread@*/
        }
-    }  break;
+       break;
 
     default:
 /*@-boundsread@*/
-       if (typeSizes[type] != -1) {
-           length = typeSizes[type] * count;
-           break;
-       }
+       if (typeSizes[type] == -1)
+           return -1;
+       length = typeSizes[(type & 0xf)] * count;
 /*@=boundsread@*/
-       /*@-modfilesys@*/
-       fprintf(stderr, _("Data type %d not supported\n"), (int) type);
-       /*@=modfilesys@*/
-       exit(EXIT_FAILURE);
-       /*@notreached@*/ break;
+       if (length < 0 || (se && (s + length) > se))
+           return -1;
+       break;
     }
 
     return length;
@@ -426,16 +422,20 @@ static int dataLength(int_32 type, hPTR_t p, int_32 count, int onDisk)
  * @param il           no. of entries
  * @param dl           start no. bytes of data
  * @param pe           header physical entry pointer (swapped)
- * @param dataStart    header data
+ * @param dataStart    header data start
+ * @param dataEnd      header data end
  * @param regionid     region offset
  * @return             no. bytes of data in region, -1 on error
  */
 static int regionSwab(/*@null@*/ indexEntry entry, int il, int dl,
-               entryInfo pe, char * dataStart, int regionid)
+               entryInfo pe,
+               unsigned char * dataStart,
+               /*@null@*/ const unsigned char * dataEnd,
+               int regionid)
        /*@modifies *entry, *dataStart @*/
 {
-    char * tprev = NULL;
-    char * t = NULL;
+    unsigned char * tprev = NULL;
+    unsigned char * t = NULL;
     int tdel, tl = dl;
     struct indexEntry_s ieprev;
 
@@ -452,8 +452,15 @@ static int regionSwab(/*@null@*/ indexEntry entry, int il, int dl,
            return -1;
        ie.info.count = ntohl(pe->count);
        ie.info.offset = ntohl(pe->offset);
+
        ie.data = t = dataStart + ie.info.offset;
-       ie.length = dataLength(ie.info.type, ie.data, ie.info.count, 1);
+       if (dataEnd && t >= dataEnd)
+           return -1;
+
+       ie.length = dataLength(ie.info.type, ie.data, ie.info.count, 1, dataEnd);
+       if (ie.length < 0 || hdrchkData(ie.length))
+           return -1;
+
        ie.rdlen = 0;
 
        if (entry) {
@@ -497,14 +504,20 @@ static int regionSwab(/*@null@*/ indexEntry entry, int il, int dl,
 /*@-bounds@*/
        case RPM_INT32_TYPE:
        {   int_32 * it = (int_32 *)t;
-           for (; ie.info.count > 0; ie.info.count--, it += 1)
+           for (; ie.info.count > 0; ie.info.count--, it += 1) {
+               if (dataEnd && ((unsigned char *)it) >= dataEnd)
+                   return -1;
                *it = htonl(*it);
+           }
            t = (char *) it;
        }   /*@switchbreak@*/ break;
        case RPM_INT16_TYPE:
        {   int_16 * it = (int_16 *) t;
-           for (; ie.info.count > 0; ie.info.count--, it += 1)
+           for (; ie.info.count > 0; ie.info.count--, it += 1) {
+               if (dataEnd && ((unsigned char *)it) >= dataEnd)
+                   return -1;
                *it = htons(*it);
+           }
            t = (char *) it;
        }   /*@switchbreak@*/ break;
 /*@=bounds@*/
@@ -686,7 +699,7 @@ t = te;
                ril++;
                rdlen += entry->info.count;
 
-               count = regionSwab(NULL, ril, 0, pe, t, 0);
+               count = regionSwab(NULL, ril, 0, pe, t, NULL, 0);
                if (count != rdlen)
                    goto errxit;
 
@@ -705,7 +718,7 @@ t = te;
                }
                te += entry->info.count + drlen;
 
-               count = regionSwab(NULL, ril, 0, pe, t, 0);
+               count = regionSwab(NULL, ril, 0, pe, t, NULL, 0);
                if (count != (rdlen + entry->info.count + drlen))
                    goto errxit;
            }
@@ -931,7 +944,8 @@ Header headerLoad(/*@kept@*/ void * uh)
     void * pv = uh;
     Header h = NULL;
     entryInfo pe;
-    char * dataStart;
+    unsigned char * dataStart;
+    unsigned char * dataEnd;
     indexEntry entry; 
     int rdlen;
     int i;
@@ -944,7 +958,8 @@ Header headerLoad(/*@kept@*/ void * uh)
     /*@-castexpose@*/
     pe = (entryInfo) &ei[2];
     /*@=castexpose@*/
-    dataStart = (char *) (pe + il);
+    dataStart = (unsigned char *) (pe + il);
+    dataEnd = dataStart + dl;
 
     h = xcalloc(1, sizeof(*h));
     /*@-assignexpose@*/
@@ -980,13 +995,13 @@ Header headerLoad(/*@kept@*/ void * uh)
        /*@-sizeoftype@*/
        entry->info.count = REGION_TAG_COUNT;
        /*@=sizeoftype@*/
-       entry->info.offset = ((char *)pe - dataStart); /* negative offset */
+       entry->info.offset = ((unsigned char *)pe - dataStart); /* negative offset */
 
        /*@-assignexpose@*/
        entry->data = pe;
        /*@=assignexpose@*/
        entry->length = pvlen - sizeof(il) - sizeof(dl);
-       rdlen = regionSwab(entry+1, il, 0, pe, dataStart, entry->info.offset);
+       rdlen = regionSwab(entry+1, il, 0, pe, dataStart, dataEnd, entry->info.offset);
 #if 0  /* XXX don't check, the 8/98 i18n bug fails here. */
        if (rdlen != dl)
            goto errxit;
@@ -995,7 +1010,6 @@ Header headerLoad(/*@kept@*/ void * uh)
        entry++;
        h->indexUsed++;
     } else {
-       int nb = ntohl(pe->count);
        int_32 rdl;
        int_32 ril;
 
@@ -1014,6 +1028,7 @@ Header headerLoad(/*@kept@*/ void * uh)
            if (hdrchkData(off))
                goto errxit;
            if (off) {
+               size_t nb = REGION_TAG_COUNT;
                int_32 * stei = memcpy(alloca(nb), dataStart + off, nb);
                rdl = -ntohl(stei[2]);  /* negative offset */
                ril = rdl/sizeof(*pe);
@@ -1034,7 +1049,7 @@ Header headerLoad(/*@kept@*/ void * uh)
        entry->data = pe;
        /*@=assignexpose@*/
        entry->length = pvlen - sizeof(il) - sizeof(dl);
-       rdlen = regionSwab(entry+1, ril-1, 0, pe+1, dataStart, entry->info.offset);
+       rdlen = regionSwab(entry+1, ril-1, 0, pe+1, dataStart, dataEnd, entry->info.offset);
        if (rdlen < 0)
            goto errxit;
        entry->rdlen = rdlen;
@@ -1046,7 +1061,7 @@ Header headerLoad(/*@kept@*/ void * uh)
            int rc;
 
            /* Load dribble entries from region. */
-           rc = regionSwab(newEntry, ne, 0, pe+ril, dataStart, rid);
+           rc = regionSwab(newEntry, ne, 0, pe+ril, dataStart, dataEnd, rid);
            if (rc < 0)
                goto errxit;
            rdlen += rc;
@@ -1380,7 +1395,7 @@ static int copyEntry(const indexEntry entry,
            /*@=sizeoftype@*/
 /*@=bounds@*/
 
-           rc = regionSwab(NULL, ril, 0, pe, dataStart, 0);
+           rc = regionSwab(NULL, ril, 0, pe, dataStart, NULL, 0);
            /* XXX 1 on success. */
            rc = (rc < 0) ? 0 : 1;
        } else {
@@ -1722,31 +1737,26 @@ int headerGetRawEntry(Header h, int_32 tag, int_32 * type, hPTR_t * p,
 /**
  */
 static void copyData(int_32 type, /*@out@*/ void * dstPtr, const void * srcPtr,
-               int_32 c, int dataLength)
+               int_32 cnt, int dataLength)
        /*@modifies *dstPtr @*/
 {
-    const char ** src;
-    char * dst;
-    int i;
-
     switch (type) {
     case RPM_STRING_ARRAY_TYPE:
     case RPM_I18NSTRING_TYPE:
-       /* Otherwise, p is char** */
-       i = c;
-       src = (const char **) srcPtr;
-       dst = dstPtr;
+    {  const char ** av = (const char **) srcPtr;
+       char * t = dstPtr;
+
 /*@-bounds@*/
-       while (i--) {
-           if (*src) {
-               int len = strlen(*src) + 1;
-               memcpy(dst, *src, len);
-               dst += len;
-           }
-           src++;
+       while (cnt-- > 0 && dataLength > 0) {
+           const char * s;
+           if ((s = *av++) == NULL)
+               continue;
+           do {
+               *t++ = *s++;
+           } while (s[-1] && --dataLength > 0);
        }
 /*@=bounds@*/
-       break;
+    }  break;
 
     default:
 /*@-boundswrite@*/
@@ -1762,17 +1772,22 @@ static void copyData(int_32 type, /*@out@*/ void * dstPtr, const void * srcPtr,
  * @param p            entry data
  * @param c            entry item count
  * @retval lengthPtr   no. bytes in returned data
- * @return             (malloc'ed) copy of entry data
+ * @return             (malloc'ed) copy of entry data, NULL on error
  */
-static void * grabData(int_32 type, hPTR_t p, int_32 c,
-               /*@out@*/ int * lengthPtr)
+/*@null@*/
+static void *
+grabData(int_32 type, hPTR_t p, int_32 c, /*@out@*/ int * lengthPtr)
        /*@modifies *lengthPtr @*/
        /*@requires maxSet(lengthPtr) >= 0 @*/
 {
-    int length = dataLength(type, p, c, 0);
-    void * data = xmalloc(length);
+    void * data = NULL;
+    int length;
 
-    copyData(type, data, p, c, length);
+    length = dataLength(type, p, c, 0, NULL);
+    if (length > 0) {
+       data = xmalloc(length);
+       copyData(type, data, p, c, length);
+    }
 
     if (lengthPtr)
        *lengthPtr = length;
@@ -1798,11 +1813,20 @@ int headerAddEntry(Header h, int_32 tag, int_32 type, const void * p, int_32 c)
        /*@modifies h @*/
 {
     indexEntry entry;
+    void * data;
+    int length;
 
     /* Count must always be >= 1 for headerAddEntry. */
     if (c <= 0)
        return 0;
 
+    length = 0;
+/*@-boundswrite@*/
+    data = grabData(type, p, c, &length);
+/*@=boundswrite@*/
+    if (data == NULL || length <= 0)
+       return 0;
+
     /* Allocate more index space if necessary */
     if (h->indexUsed == h->indexAlloced) {
        h->indexAlloced += INDEX_MALLOC_SIZE;
@@ -1815,9 +1839,8 @@ int headerAddEntry(Header h, int_32 tag, int_32 type, const void * p, int_32 c)
     entry->info.type = type;
     entry->info.count = c;
     entry->info.offset = 0;
-/*@-boundswrite@*/
-    entry->data = grabData(type, p, c, &entry->length);
-/*@=boundswrite@*/
+    entry->data = data;
+    entry->length = length;
 
 /*@-boundsread@*/
     if (h->indexUsed > 0 && tag < h->index[h->indexUsed-1].info.tag)
@@ -1860,7 +1883,7 @@ int headerAppendEntry(Header h, int_32 tag, int_32 type,
     if (!entry)
        return 0;
 
-    length = dataLength(type, p, c, 0);
+    length = dataLength(type, p, c, 0, NULL);
 
     if (ENTRY_IN_REGION(entry)) {
        char * t = xmalloc(entry->length + length);
@@ -2073,12 +2096,19 @@ int headerModifyEntry(Header h, int_32 tag, int_32 type,
 {
     indexEntry entry;
     void * oldData;
+    void * data;
+    int length;
 
     /* First find the tag */
     entry = findEntry(h, tag, type);
     if (!entry)
        return 0;
 
+    length = 0;
+    data = grabData(type, p, c, &length);
+    if (data == NULL || length <= 0)
+       return 0;
+
     /* make sure entry points to the first occurence of this tag */
     while (entry > h->index && (entry - 1)->info.tag == tag)  
        entry--;
@@ -2089,7 +2119,8 @@ int headerModifyEntry(Header h, int_32 tag, int_32 type,
 
     entry->info.count = c;
     entry->info.type = type;
-    entry->data = grabData(type, p, c, &entry->length);
+    entry->data = data;
+    entry->length = length;
 
     /*@-branchstate@*/
     if (ENTRY_IN_REGION(entry)) {

diff --git a/rpmio/rpmpgp.h b/rpmio/rpmpgp.h
index 04e3768..defd6fd 100644 (file)
--- a/rpmio/rpmpgp.h
+++ b/rpmio/rpmpgp.h
@@ -1209,7 +1209,7 @@ int pgpPrtPkt(const byte *pkt, unsigned int pleft)
 /**
  * Print/parse a OpenPGP packet(s).
  * @param pkts         OpenPGP packet(s)
- * @param pktlen               OpenPGP packet(s) length (no. of bytes)
+ * @param pktlen       OpenPGP packet(s) length (no. of bytes)
  * @retval dig         parsed output of signature/pubkey packet parameters
  * @param printing     should packets be printed?
  * @return             -1 on error, 0 on success