Start eliminating static buffers from header/signature checks

- Push msg buffer allocations down to the lowlevel rpmVerifySignature() and
  internal verify*Signature functions, nothing above them knows how much
  memory they need for messages.  Use rasprintf() where obvious,
  stupid malloc(bigenuf) for now otherwise.
- Changes public API but can't be helped, printing to an unchecked buffer(s)
  of unknown size from one of the more security sensitive pieces is just
  .. not ok
- Minimally convert callers to the new allocation scheme

diff --git a/lib/package.c b/lib/package.c
index 2102759..b0f2831 100644 (file)
--- a/lib/package.c
+++ b/lib/package.c
@@ -307,7 +307,7 @@ int headerVerifyInfo(int il, int dl, const void * pev, void * iv, int negate)
 rpmRC headerCheck(rpmts ts, const void * uh, size_t uc, char ** msg)
 {
     pgpDig dig;
-    char buf[8*BUFSIZ];
+    char *buf = NULL;
     int32_t * ei = (int32_t *) uh;
     int32_t il = ntohl(ei[0]);
     int32_t dl = ntohl(ei[1]);
@@ -331,12 +331,10 @@ rpmRC headerCheck(rpmts ts, const void * uh, size_t uc, char ** msg)
     static int hclvl;
 
     hclvl++;
-    buf[0] = '\0';
 
     /* Is the blob the right size? */
     if (uc > 0 && pvlen != uc) {
-       (void) snprintf(buf, sizeof(buf),
-               _("blob size(%d): BAD, 8 + 16 * il(%d) + dl(%d)\n"),
+       rasprintf(&buf, _("blob size(%d): BAD, 8 + 16 * il(%d) + dl(%d)\n"),
                (int)uc, (int)il, (int)dl);
        goto exit;
     }
@@ -344,8 +342,7 @@ rpmRC headerCheck(rpmts ts, const void * uh, size_t uc, char ** msg)
     /* Check (and convert) the 1st tag element. */
     xx = headerVerifyInfo(1, dl, pe, &entry->info, 0);
     if (xx != -1) {
-       (void) snprintf(buf, sizeof(buf),
-               _("tag[%d]: BAD, tag %d type %d offset %d count %d\n"),
+       rasprintf(&buf, _("tag[%d]: BAD, tag %d type %d offset %d count %d\n"),
                0, entry->info.tag, entry->info.type,
                entry->info.offset, entry->info.count);
        goto exit;
@@ -362,7 +359,7 @@ rpmRC headerCheck(rpmts ts, const void * uh, size_t uc, char ** msg)
 
     /* Is the offset within the data area? */
     if (entry->info.offset >= dl) {
-       (void) snprintf(buf, sizeof(buf),
+       rasprintf(&buf, 
                _("region offset: BAD, tag %d type %d offset %d count %d\n"),
                entry->info.tag, entry->info.type,
                entry->info.offset, entry->info.count);
@@ -380,7 +377,7 @@ rpmRC headerCheck(rpmts ts, const void * uh, size_t uc, char ** msg)
        && entry->info.type == RPM_BIN_TYPE
        && entry->info.count == REGION_TAG_COUNT))
     {
-       (void) snprintf(buf, sizeof(buf),
+       rasprintf(&buf, 
                _("region trailer: BAD, tag %d type %d offset %d count %d\n"),
                entry->info.tag, entry->info.type,
                entry->info.offset, entry->info.count);
@@ -391,8 +388,7 @@ rpmRC headerCheck(rpmts ts, const void * uh, size_t uc, char ** msg)
     /* Is the no. of tags in the region less than the total no. of tags? */
     ril = entry->info.offset/sizeof(*pe);
     if ((entry->info.offset % sizeof(*pe)) || ril > il) {
-       (void) snprintf(buf, sizeof(buf),
-               _("region size: BAD, ril(%d) > il(%d)\n"), ril, il);
+       rasprintf(&buf, _("region size: BAD, ril(%d) > il(%d)\n"), ril, il);
        goto exit;
     }
 
@@ -400,7 +396,7 @@ rpmRC headerCheck(rpmts ts, const void * uh, size_t uc, char ** msg)
     for (i = ril; i < il; i++) {
        xx = headerVerifyInfo(1, dl, pe+i, &entry->info, 0);
        if (xx != -1) {
-           (void) snprintf(buf, sizeof(buf),
+           rasprintf(&buf,
                _("tag[%d]: BAD, tag %d type %d offset %d count %d\n"),
                i, entry->info.tag, entry->info.type,
                entry->info.offset, entry->info.count);
@@ -419,7 +415,7 @@ rpmRC headerCheck(rpmts ts, const void * uh, size_t uc, char ** msg)
            }
            if (entry->info.type != RPM_STRING_TYPE || *b != '\0' || blen != 40)
            {
-               (void) snprintf(buf, sizeof(buf), _("hdr SHA1: BAD, not hex\n"));
+               rasprintf(&buf, _("hdr SHA1: BAD, not hex\n"));
                goto exit;
            }
            if (info->tag == 0) {
@@ -431,7 +427,7 @@ rpmRC headerCheck(rpmts ts, const void * uh, size_t uc, char ** msg)
            if (vsflags & RPMVSF_NORSAHEADER)
                break;
            if (entry->info.type != RPM_BIN_TYPE) {
-               (void) snprintf(buf, sizeof(buf), _("hdr RSA: BAD, not binary\n"));
+               rasprintf(&buf, _("hdr RSA: BAD, not binary\n"));
                goto exit;
            }
            *info = entry->info;        /* structure assignment */
@@ -441,7 +437,7 @@ rpmRC headerCheck(rpmts ts, const void * uh, size_t uc, char ** msg)
            if (vsflags & RPMVSF_NODSAHEADER)
                break;
            if (entry->info.type != RPM_BIN_TYPE) {
-               (void) snprintf(buf, sizeof(buf), _("hdr DSA: BAD, not binary\n"));
+               rasprintf(&buf, _("hdr DSA: BAD, not binary\n"));
                goto exit;
            }
            *info = entry->info;        /* structure assignment */
@@ -456,8 +452,10 @@ rpmRC headerCheck(rpmts ts, const void * uh, size_t uc, char ** msg)
 exit:
     /* Return determined RPMRC_OK/RPMRC_FAIL conditions. */
     if (rc != RPMRC_NOTFOUND) {
-       buf[sizeof(buf)-1] = '\0';
-       if (msg) *msg = xstrdup(buf);
+       if (msg) 
+           *msg = buf;
+       else
+           free(buf);
        hclvl--;
        return rc;
     }
@@ -467,17 +465,19 @@ exit:
 verifyinfo_exit:
        xx = headerVerifyInfo(ril-1, dl, pe+1, &entry->info, 0);
        if (xx != -1) {
-           (void) snprintf(buf, sizeof(buf),
+           rasprintf(&buf,
                _("tag[%d]: BAD, tag %d type %d offset %d count %d\n"),
                xx+1, entry->info.tag, entry->info.type,
                entry->info.offset, entry->info.count);
            rc = RPMRC_FAIL;
        } else {
-           (void) snprintf(buf, sizeof(buf), "Header sanity check: OK\n");
+           rasprintf(&buf, "Header sanity check: OK\n");
            rc = RPMRC_OK;
        }
-       buf[sizeof(buf)-1] = '\0';
-       if (msg) *msg = xstrdup(buf);
+       if (msg) 
+           *msg = buf;
+       else
+           free(buf);
        hclvl--;
        return rc;
     }
@@ -579,11 +579,12 @@ verifyinfo_exit:
        break;
     }
 
-    buf[0] = '\0';
-    rc = rpmVerifySignature(ts, buf);
+    rc = rpmVerifySignature(ts, &buf);
 
-    buf[sizeof(buf)-1] = '\0';
-    if (msg) *msg = xstrdup(buf);
+    if (msg) 
+       *msg = buf;
+    else
+       free(buf);
 
     /* XXX headerCheck can recurse, free info only at top level. */
     if (hclvl == 1)
@@ -910,24 +911,23 @@ rpmRC rpmReadPackageFile(rpmts ts, FD_t fd, const char * fn, Header * hdrp)
 
 /** @todo Implement disable/enable/warn/error/anal policy. */
 
-    buf[0] = '\0';
-    rc = rpmVerifySignature(ts, buf);
+    rc = rpmVerifySignature(ts, &msg);
     switch (rc) {
     case RPMRC_OK:             /* Signature is OK. */
-       rpmlog(RPMLOG_DEBUG, "%s: %s", fn, buf);
+       rpmlog(RPMLOG_DEBUG, "%s: %s", fn, msg);
        break;
     case RPMRC_NOTTRUSTED:     /* Signature is OK, but key is not trusted. */
     case RPMRC_NOKEY:          /* Public key is unavailable. */
        /* XXX Print NOKEY/NOTTRUSTED warning only once. */
     {  int lvl = (rpmtsStashKeyid(ts) ? RPMLOG_DEBUG : RPMLOG_WARNING);
-       rpmlog(lvl, "%s: %s", fn, buf);
+       rpmlog(lvl, "%s: %s", fn, msg);
     }  break;
     case RPMRC_NOTFOUND:       /* Signature is unknown type. */
-       rpmlog(RPMLOG_WARNING, "%s: %s", fn, buf);
+       rpmlog(RPMLOG_WARNING, "%s: %s", fn, msg);
        break;
     default:
     case RPMRC_FAIL:           /* Signature does not verify. */
-       rpmlog(RPMLOG_ERR, "%s: %s", fn, buf);
+       rpmlog(RPMLOG_ERR, "%s: %s", fn, msg);
        break;
     }
 

diff --git a/lib/rpmchecksig.c b/lib/rpmchecksig.c
index cba9ba6..1470d47 100644 (file)
--- a/lib/rpmchecksig.c
+++ b/lib/rpmchecksig.c
@@ -558,7 +558,6 @@ static const char *sigtagname(rpmSigTag sigtag, int upper)
 int rpmVerifySignatures(QVA_t qva, rpmts ts, FD_t fd,
                const char * fn)
 {
-    char result[1024];
     char buf[8192], * b;
     char missingKeys[7164], * m;
     char untrustedKeys[7164], * u;
@@ -672,6 +671,7 @@ int rpmVerifySignatures(QVA_t qva, rpmts ts, FD_t fd,
            headerNextIterator(hi, &sigtag, &sigtype, &sig, &siglen) != 0;
            (void) rpmtsSetSig(ts, sigtag, sigtype, NULL, siglen))
        {
+           char *result = NULL;
 
            if (sig == NULL) /* XXX can't happen */
                continue;
@@ -724,7 +724,7 @@ int rpmVerifySignatures(QVA_t qva, rpmts ts, FD_t fd,
                break;
            }
 
-           sigres = rpmVerifySignature(ts, result);
+           sigres = rpmVerifySignature(ts, &result);
            if (sigres != RPMRC_OK) {
                failed = 1;
            }
@@ -789,6 +789,7 @@ int rpmVerifySignatures(QVA_t qva, rpmts ts, FD_t fd,
                    break;
                }
            }
+           free(result);
        }
        hi = headerFreeIterator(hi);
 

diff --git a/lib/rpmlib.h b/lib/rpmlib.h
index c7bea3d..5f7039e 100644 (file)
--- a/lib/rpmlib.h
+++ b/lib/rpmlib.h
@@ -251,10 +251,10 @@ int rpmGetFilesystemUsage(const char ** fileList, rpm_off_t * fssizes,
  *
  * @param ts           transaction set
  * @retval result      detailed text result of signature verification
+ *                     (malloc'd)
  * @return             result of signature verification
  */
-rpmRC rpmVerifySignature(const rpmts ts,
-               char * result);
+rpmRC rpmVerifySignature(const rpmts ts, char ** result);
 
 /** \ingroup signature
  * Destroy signature header from package.

diff --git a/lib/signature.c b/lib/signature.c
index 30d20d1..ddf947f 100644 (file)
--- a/lib/signature.c
+++ b/lib/signature.c
@@ -943,14 +943,17 @@ static const char * rpmSigString(rpmRC res)
 }
 
 static rpmRC
-verifySizeSignature(const rpmts ts, char * t)
+verifySizeSignature(const rpmts ts, char ** msg)
 {
     rpm_constdata_t sig = rpmtsSig(ts);
     pgpDig dig = rpmtsDig(ts);
     rpmRC res;
     size_t size = 0x7fffffff;
+    char *t;
+
+    *msg = xmalloc(BUFSIZ); /* XXX FIXME, calculate string size instead */
+    t = *msg;
 
-    *t = '\0';
     t = stpcpy(t, _("Header+Payload size: "));
 
     if (sig == NULL || dig == NULL || dig->nbytes == 0) {
@@ -977,7 +980,7 @@ exit:
 }
 
 static rpmRC
-verifyMD5Signature(const rpmts ts, char * t,
+verifyMD5Signature(const rpmts ts, char ** msg,
                DIGEST_CTX md5ctx)
 {
     rpm_constdata_t sig = rpmtsSig(ts);
@@ -986,8 +989,11 @@ verifyMD5Signature(const rpmts ts, char * t,
     rpmRC res;
     uint8_t * md5sum = NULL;
     size_t md5len = 0;
+    char * t;
+
+    *msg = xmalloc(BUFSIZ); /* XXX FIXME, calculate string size instead */
+    t = *msg;
 
-    *t = '\0';
     t = stpcpy(t, _("MD5 digest: "));
 
     if (md5ctx == NULL || sig == NULL || dig == NULL) {
@@ -1027,12 +1033,12 @@ exit:
 /**
  * Verify header immutable region SHA1 digest.
  * @param ts           transaction set
- * @retval t           verbose success/failure text
+ * @retval msg         rbose success/failure text
  * @param sha1ctx
  * @return             RPMRC_OK on success
  */
 static rpmRC
-verifySHA1Signature(const rpmts ts, char * t,
+verifySHA1Signature(const rpmts ts, char ** msg,
                DIGEST_CTX sha1ctx)
 {
     rpm_constdata_t sig = rpmtsSig(ts);
@@ -1042,8 +1048,11 @@ verifySHA1Signature(const rpmts ts, char * t,
     pgpDig dig = rpmtsDig(ts);
     rpmRC res;
     char * SHA1 = NULL;
+    char *t;
+
+    *msg = xmalloc(BUFSIZ); /* XXX FIXME, calculate string size instead */
+    t = *msg;
 
-    *t = '\0';
     t = stpcpy(t, _("Header SHA1 digest: "));
 
     if (sha1ctx == NULL || sig == NULL || dig == NULL) {
@@ -1097,12 +1106,12 @@ static inline unsigned char nibble(char c)
 /**
  * Verify RSA signature.
  * @param ts           transaction set
- * @retval t           verbose success/failure text
+ * @retval msg         rbose success/failure text
  * @param md5ctx
  * @return             RPMRC_OK on success
  */
 static rpmRC
-verifyRSASignature(rpmts ts, char * t,
+verifyRSASignature(rpmts ts, char ** msg,
                DIGEST_CTX md5ctx)
 {
     rpm_constdata_t sig = rpmtsSig(ts);
@@ -1116,8 +1125,11 @@ verifyRSASignature(rpmts ts, char * t,
     rpmRC res = RPMRC_OK;
     int xx;
     SECItem digest;
+    char *t;
+
+    *msg = xmalloc(BUFSIZ); /* XXX FIXME, calculate string size instead */
+    t = *msg;
 
-    *t = '\0';
     if (dig != NULL && dig->hdrmd5ctx == md5ctx)
        t = stpcpy(t, _("Header "));
     *t++ = 'V';
@@ -1250,7 +1262,7 @@ exit:
  * @return             RPMRC_OK on success
  */
 static rpmRC
-verifyDSASignature(rpmts ts, char * t,
+verifyDSASignature(rpmts ts, char ** msg,
                DIGEST_CTX sha1ctx)
 {
     rpm_constdata_t sig = rpmtsSig(ts);
@@ -1263,8 +1275,11 @@ verifyDSASignature(rpmts ts, char * t,
     rpmRC res;
     int xx;
     SECItem digest;
+    char *t;
+
+    *msg = xmalloc(BUFSIZ); /* XXX FIXME, calculate string size instead */
+    t = *msg;
 
-    *t = '\0';
     if (dig != NULL && dig->hdrsha1ctx == sha1ctx)
        t = stpcpy(t, _("Header "));
     *t++ = 'V';
@@ -1342,7 +1357,7 @@ exit:
 }
 
 rpmRC
-rpmVerifySignature(const rpmts ts, char * result)
+rpmVerifySignature(const rpmts ts, char ** result)
 {
     rpm_constdata_t sig = rpmtsSig(ts);
     size_t siglen = rpmtsSiglen(ts);
@@ -1350,8 +1365,10 @@ rpmVerifySignature(const rpmts ts, char * result)
     pgpDig dig = rpmtsDig(ts);
     rpmRC res;
 
+    assert(result != NULL);
+
     if (sig == NULL || siglen <= 0 || dig == NULL) {
-       sprintf(result, _("Verify signature: BAD PARAMETERS\n"));
+       rasprintf(result, _("Verify signature: BAD PARAMETERS\n"));
        return RPMRC_NOTFOUND;
     }
 
@@ -1382,11 +1399,11 @@ rpmVerifySignature(const rpmts ts, char * result)
        break;
     case RPMSIGTAG_LEMD5_1:
     case RPMSIGTAG_LEMD5_2:
-       sprintf(result, _("Broken MD5 digest: UNSUPPORTED\n"));
+       rasprintf(result, _("Broken MD5 digest: UNSUPPORTED\n"));
        res = RPMRC_NOTFOUND;
        break;
     default:
-       sprintf(result, _("Signature: UNKNOWN (%d)\n"), sigtag);
+       rasprintf(result, _("Signature: UNKNOWN (%d)\n"), sigtag);
        res = RPMRC_NOTFOUND;
        break;
     }