rdelaage
/
postgresql
mirror of https://git.postgresql.org/git/postgresql.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix Coverity warning about contrib/pgcrypto's mdc_finish(). 

Coverity points out that mdc_finish returns a pointer to a local buffer
(which of course is gone as soon as the function returns), leaving open
a risk of misbehaviors possibly as bad as a stack overwrite.

In reality, the only possible call site is in process_data_packets()
which does not examine the returned pointer at all.  So there's no
live bug, but nonetheless the code is confusing and risky.  Refactor
to avoid the issue by letting process_data_packets() call mdc_finish()
directly instead of going through the pullf_read() API.

Although this is only cosmetic, it seems good to back-patch so that
the logic in pgp-decrypt.c stays in sync across all branches.

Marko Kreen
This commit is contained in:
Tom Lane 2015-01-30 13:04:56 -05:00
parent bd4e2fd97d
commit a59ee88197
1 changed files with 19 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

49
contrib/pgcrypto/pgp-decrypt.c
View File

@ -351,37 +351,33 @@ mdc_free(void *priv)
} }
static int static int
mdc_finish(PGP_Context *ctx, PullFilter *src, mdc_finish(PGP_Context *ctx, PullFilter *src, int len)
int len, uint8 **data_p)
{ {
int res; int res;
uint8 hash[20]; uint8 hash[20];
uint8 tmpbuf[22]; uint8 tmpbuf[20];
uint8 *data;
if (len + 1 > sizeof(tmpbuf)) /* should not happen */
if (ctx->use_mdcbuf_filter)
return PXE_BUG; return PXE_BUG;
/* It's SHA1 */
if (len != 20)
return PXE_PGP_CORRUPT_DATA;
/* mdc_read should not call md_update */
ctx->in_mdc_pkt = 1;
/* read data */ /* read data */
res = pullf_read_max(src, len + 1, data_p, tmpbuf); res = pullf_read_max(src, len, &data, tmpbuf);
if (res < 0) if (res < 0)
return res; return res;
if (res == 0) if (res == 0)
{ {
if (ctx->mdc_checked == 0) px_debug("no mdc");
{
px_debug("no mdc");
return PXE_PGP_CORRUPT_DATA;
}
return 0;
}
/* safety check */
if (ctx->in_mdc_pkt > 1)
{
px_debug("mdc_finish: several times here?");
return PXE_PGP_CORRUPT_DATA; return PXE_PGP_CORRUPT_DATA;
} }
ctx->in_mdc_pkt++;
/* is the packet sane? */ /* is the packet sane? */
if (res != 20) if (res != 20)
@ -394,7 +390,7 @@ mdc_finish(PGP_Context *ctx, PullFilter *src,
* ok, we got the hash, now check * ok, we got the hash, now check
*/ */
px_md_finish(ctx->mdc_ctx, hash); px_md_finish(ctx->mdc_ctx, hash);
res = memcmp(hash, *data_p, 20); res = memcmp(hash, data, 20);
px_memset(hash, 0, 20); px_memset(hash, 0, 20);
px_memset(tmpbuf, 0, sizeof(tmpbuf)); px_memset(tmpbuf, 0, sizeof(tmpbuf));
if (res != 0) if (res != 0)
@ -403,7 +399,7 @@ mdc_finish(PGP_Context *ctx, PullFilter *src,
return PXE_PGP_CORRUPT_DATA; return PXE_PGP_CORRUPT_DATA;
} }
ctx->mdc_checked = 1; ctx->mdc_checked = 1;
return len; return 0;
} }
static int static int
@ -414,12 +410,9 @@ mdc_read(void *priv, PullFilter *src, int len,
PGP_Context *ctx = priv; PGP_Context *ctx = priv;
/* skip this filter? */ /* skip this filter? */
if (ctx->use_mdcbuf_filter) if (ctx->use_mdcbuf_filter || ctx->in_mdc_pkt)
return pullf_read(src, len, data_p); return pullf_read(src, len, data_p);
if (ctx->in_mdc_pkt)
return mdc_finish(ctx, src, len, data_p);
res = pullf_read(src, len, data_p); res = pullf_read(src, len, data_p);
if (res < 0) if (res < 0)
return res; return res;
@ -878,7 +871,6 @@ process_data_packets(PGP_Context *ctx, MBuf *dst, PullFilter *src,
int got_data = 0; int got_data = 0;
int got_mdc = 0; int got_mdc = 0;
PullFilter *pkt = NULL; PullFilter *pkt = NULL;
uint8 *tmp;
while (1) while (1)
{ {
@ -937,11 +929,8 @@ process_data_packets(PGP_Context *ctx, MBuf *dst, PullFilter *src,
break; break;
} }
/* notify mdc_filter */ res = mdc_finish(ctx, pkt, len);
ctx->in_mdc_pkt = 1; if (res >= 0)
res = pullf_read(pkt, 8192, &tmp);
if (res > 0)
got_mdc = 1; got_mdc = 1;
break; break;
default: default: