Skip to content

Commit df2e626

Browse files
bmomjianbmomjian
committed
pgcrypto: fix memset() calls that might be optimized away
Specifically, on-stack memset() might be removed, so: * Replace memset() with px_memset() * Add px_memset to copy_crlf() * Add px_memset to pgp-s2k.c Patch by Marko Kreen Report by PVS-Studio Backpatch through 8.4.
1 parent 7a9bf93 commit df2e626

22 files changed

+81
-68
lines changed

‎contrib/pgcrypto/crypt-blowfish.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,8 @@
3434

3535
#include "postgres.h"
3636

37-
#include "px.h"
3837
#include "px-crypt.h"
38+
#include "px.h"
3939

4040
#ifdef __i386__
4141
#define BF_ASM 0 /* 1 */
@@ -617,7 +617,7 @@ _crypt_blowfish_rn(const char *key, const char *setting,
617617
count = (BF_word) 1 << ((setting[4] - '0') * 10 + (setting[5] - '0'));
618618
if (count < 16 || BF_decode(data.binary.salt, &setting[7], 16))
619619
{
620-
memset(data.binary.salt, 0, sizeof(data.binary.salt));
620+
px_memset(data.binary.salt, 0, sizeof(data.binary.salt));
621621
return NULL;
622622
}
623623
BF_swap(data.binary.salt, 4);
@@ -730,7 +730,7 @@ _crypt_blowfish_rn(const char *key, const char *setting,
730730
/* Overwrite the most obvious sensitive data we have on the stack. Note
731731
* that this does not guarantee there's no sensitive data left on the
732732
* stack and/or in registers; I'm not aware of portable code that does. */
733-
memset(&data, 0, sizeof(data));
733+
px_memset(&data, 0, sizeof(data));
734734

735735
return output;
736736
}

‎contrib/pgcrypto/crypt-md5.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ px_crypt_md5(const char *pw, const char *salt, char *passwd, unsigned dstlen)
8989
px_md_update(ctx, final, pl > MD5_SIZE ? MD5_SIZE : pl);
9090

9191
/* Don't leave anything around in vm they could use. */
92-
memset(final, 0, sizeof final);
92+
px_memset(final, 0, sizeof final);
9393

9494
/* Then something really weird... */
9595
for (i = strlen(pw); i; i >>= 1)
@@ -154,7 +154,7 @@ px_crypt_md5(const char *pw, const char *salt, char *passwd, unsigned dstlen)
154154
*p = '\0';
155155

156156
/* Don't leave anything around in vm they could use. */
157-
memset(final, 0, sizeof final);
157+
px_memset(final, 0, sizeof final);
158158

159159
px_md_free(ctx1);
160160
px_md_free(ctx);

‎contrib/pgcrypto/fortuna.c

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
#include <sys/time.h>
3535
#include <time.h>
3636

37+
#include "px.h"
3738
#include "rijndael.h"
3839
#include "sha2.h"
3940
#include "fortuna.h"
@@ -169,7 +170,7 @@ md_result(MD_CTX * ctx, uint8 *dst)
169170

170171
memcpy(&tmp, ctx, sizeof(*ctx));
171172
SHA256_Final(dst, &tmp);
172-
memset(&tmp, 0, sizeof(tmp));
173+
px_memset(&tmp, 0, sizeof(tmp));
173174
}
174175

175176
/*
@@ -243,7 +244,7 @@ enough_time_passed(FState *st)
243244
if (ok)
244245
memcpy(last, &tv, sizeof(tv));
245246

246-
memset(&tv, 0, sizeof(tv));
247+
px_memset(&tv, 0, sizeof(tv));
247248

248249
return ok;
249250
}
@@ -290,8 +291,8 @@ reseed(FState *st)
290291
/* use new key */
291292
ciph_init(&st->ciph, st->key, BLOCK);
292293

293-
memset(&key_md, 0, sizeof(key_md));
294-
memset(buf, 0, BLOCK);
294+
px_memset(&key_md, 0, sizeof(key_md));
295+
px_memset(buf, 0, BLOCK);
295296
}
296297

297298
/*
@@ -341,8 +342,8 @@ add_entropy(FState *st, const uint8 *data, unsigned len)
341342
if (pos == 0)
342343
st->pool0_bytes += len;
343344

344-
memset(hash, 0, BLOCK);
345-
memset(&md, 0, sizeof(md));
345+
px_memset(hash, 0, BLOCK);
346+
px_memset(&md, 0, sizeof(md));
346347
}
347348

348349
/*
@@ -378,7 +379,7 @@ startup_tricks(FState *st)
378379
encrypt_counter(st, buf + CIPH_BLOCK);
379380
md_update(&st->pool[i], buf, BLOCK);
380381
}
381-
memset(buf, 0, BLOCK);
382+
px_memset(buf, 0, BLOCK);
382383

383384
/* Hide the key. */
384385
rekey(st);

‎contrib/pgcrypto/internal-sha2.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ int_sha224_free(PX_MD *h)
8484
{
8585
SHA224_CTX *ctx = (SHA224_CTX *) h->p.ptr;
8686

87-
memset(ctx, 0, sizeof(*ctx));
87+
px_memset(ctx, 0, sizeof(*ctx));
8888
px_free(ctx);
8989
px_free(h);
9090
}
@@ -132,7 +132,7 @@ int_sha256_free(PX_MD *h)
132132
{
133133
SHA256_CTX *ctx = (SHA256_CTX *) h->p.ptr;
134134

135-
memset(ctx, 0, sizeof(*ctx));
135+
px_memset(ctx, 0, sizeof(*ctx));
136136
px_free(ctx);
137137
px_free(h);
138138
}
@@ -180,7 +180,7 @@ int_sha384_free(PX_MD *h)
180180
{
181181
SHA384_CTX *ctx = (SHA384_CTX *) h->p.ptr;
182182

183-
memset(ctx, 0, sizeof(*ctx));
183+
px_memset(ctx, 0, sizeof(*ctx));
184184
px_free(ctx);
185185
px_free(h);
186186
}
@@ -228,7 +228,7 @@ int_sha512_free(PX_MD *h)
228228
{
229229
SHA512_CTX *ctx = (SHA512_CTX *) h->p.ptr;
230230

231-
memset(ctx, 0, sizeof(*ctx));
231+
px_memset(ctx, 0, sizeof(*ctx));
232232
px_free(ctx);
233233
px_free(h);
234234
}

‎contrib/pgcrypto/internal.c

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ int_md5_free(PX_MD *h)
143143
{
144144
MD5_CTX *ctx = (MD5_CTX *) h->p.ptr;
145145

146-
memset(ctx, 0, sizeof(*ctx));
146+
px_memset(ctx, 0, sizeof(*ctx));
147147
px_free(ctx);
148148
px_free(h);
149149
}
@@ -191,7 +191,7 @@ int_sha1_free(PX_MD *h)
191191
{
192192
SHA1_CTX *ctx = (SHA1_CTX *) h->p.ptr;
193193

194-
memset(ctx, 0, sizeof(*ctx));
194+
px_memset(ctx, 0, sizeof(*ctx));
195195
px_free(ctx);
196196
px_free(h);
197197
}
@@ -266,7 +266,7 @@ intctx_free(PX_Cipher *c)
266266

267267
if (cx)
268268
{
269-
memset(cx, 0, sizeof *cx);
269+
px_memset(cx, 0, sizeof *cx);
270270
px_free(cx);
271271
}
272272
px_free(c);
@@ -659,7 +659,7 @@ system_reseed(void)
659659
skip = buf[0] >= SYSTEM_RESEED_CHANCE;
660660
}
661661
/* clear 1 byte */
662-
memset(buf, 0, sizeof(buf));
662+
px_memset(buf, 0, sizeof(buf));
663663

664664
if (skip)
665665
return;
@@ -669,7 +669,7 @@ system_reseed(void)
669669
fortuna_add_entropy(buf, n);
670670

671671
seed_time = t;
672-
memset(buf, 0, sizeof(buf));
672+
px_memset(buf, 0, sizeof(buf));
673673
}
674674

675675
int

‎contrib/pgcrypto/mbuf.c

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ mbuf_free(MBuf *mbuf)
6969
{
7070
if (mbuf->own_data)
7171
{
72-
memset(mbuf->data, 0, mbuf->buf_end - mbuf->data);
72+
px_memset(mbuf->data, 0, mbuf->buf_end - mbuf->data);
7373
px_free(mbuf->data);
7474
}
7575
px_free(mbuf);
@@ -249,11 +249,11 @@ pullf_free(PullFilter *pf)
249249

250250
if (pf->buf)
251251
{
252-
memset(pf->buf, 0, pf->buflen);
252+
px_memset(pf->buf, 0, pf->buflen);
253253
px_free(pf->buf);
254254
}
255255

256-
memset(pf, 0, sizeof(*pf));
256+
px_memset(pf, 0, sizeof(*pf));
257257
px_free(pf);
258258
}
259259

@@ -298,7 +298,7 @@ pullf_read_max(PullFilter *pf, int len, uint8 **data_p, uint8 *tmpbuf)
298298
if (res < 0)
299299
{
300300
/* so the caller must clear only on success */
301-
memset(tmpbuf, 0, total);
301+
px_memset(tmpbuf, 0, total);
302302
return res;
303303
}
304304
if (res == 0)
@@ -415,11 +415,11 @@ pushf_free(PushFilter *mp)
415415

416416
if (mp->buf)
417417
{
418-
memset(mp->buf, 0, mp->block_size);
418+
px_memset(mp->buf, 0, mp->block_size);
419419
px_free(mp->buf);
420420
}
421421

422-
memset(mp, 0, sizeof(*mp));
422+
px_memset(mp, 0, sizeof(*mp));
423423
px_free(mp);
424424
}
425425

‎contrib/pgcrypto/openssl.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ EVP_MD_CTX_init(EVP_MD_CTX *ctx)
142142
static int
143143
EVP_MD_CTX_cleanup(EVP_MD_CTX *ctx)
144144
{
145-
memset(ctx, 0, sizeof(*ctx));
145+
px_memset(ctx, 0, sizeof(*ctx));
146146
return 1;
147147
}
148148

@@ -381,7 +381,7 @@ gen_ossl_free(PX_Cipher *c)
381381
{
382382
ossldata *od = (ossldata *) c->ptr;
383383

384-
memset(od, 0, sizeof(*od));
384+
px_memset(od, 0, sizeof(*od));
385385
px_free(od);
386386
px_free(c);
387387
}

‎contrib/pgcrypto/pgp-cfb.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ void
8585
pgp_cfb_free(PGP_CFB *ctx)
8686
{
8787
px_cipher_free(ctx->ciph);
88-
memset(ctx, 0, sizeof(*ctx));
88+
px_memset(ctx, 0, sizeof(*ctx));
8989
px_free(ctx);
9090
}
9191

‎contrib/pgcrypto/pgp-compress.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ compress_free(void *priv)
175175
struct ZipStat *st = priv;
176176

177177
deflateEnd(&st->stream);
178-
memset(st, 0, sizeof(*st));
178+
px_memset(st, 0, sizeof(*st));
179179
px_free(st);
180180
}
181181

@@ -298,7 +298,7 @@ decompress_free(void *priv)
298298
struct DecomprData *dec = priv;
299299

300300
inflateEnd(&dec->stream);
301-
memset(dec, 0, sizeof(*dec));
301+
px_memset(dec, 0, sizeof(*dec));
302302
px_free(dec);
303303
}
304304

‎contrib/pgcrypto/pgp-decrypt.c

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ pktreader_free(void *priv)
210210
{
211211
struct PktData *pkt = priv;
212212

213-
memset(pkt, 0, sizeof(*pkt));
213+
px_memset(pkt, 0, sizeof(*pkt));
214214
px_free(pkt);
215215
}
216216

@@ -257,7 +257,7 @@ prefix_init(void **priv_p, void *arg, PullFilter *src)
257257
if (res != len + 2)
258258
{
259259
px_debug("prefix_init: short read");
260-
memset(tmpbuf, 0, sizeof(tmpbuf));
260+
px_memset(tmpbuf, 0, sizeof(tmpbuf));
261261
return PXE_PGP_CORRUPT_DATA;
262262
}
263263

@@ -280,7 +280,7 @@ prefix_init(void **priv_p, void *arg, PullFilter *src)
280280
*/
281281
ctx->corrupt_prefix = 1;
282282
}
283-
memset(tmpbuf, 0, sizeof(tmpbuf));
283+
px_memset(tmpbuf, 0, sizeof(tmpbuf));
284284
return 0;
285285
}
286286

@@ -395,8 +395,8 @@ mdc_finish(PGP_Context *ctx, PullFilter *src,
395395
*/
396396
px_md_finish(ctx->mdc_ctx, hash);
397397
res = memcmp(hash, *data_p, 20);
398-
memset(hash, 0, 20);
399-
memset(tmpbuf, 0, sizeof(tmpbuf));
398+
px_memset(hash, 0, 20);
399+
px_memset(tmpbuf, 0, sizeof(tmpbuf));
400400
if (res != 0)
401401
{
402402
px_debug("mdc_finish: mdc failed");
@@ -493,7 +493,7 @@ mdcbuf_finish(struct MDCBufData * st)
493493
px_md_update(st->ctx->mdc_ctx, st->mdc_buf, 2);
494494
px_md_finish(st->ctx->mdc_ctx, hash);
495495
res = memcmp(hash, st->mdc_buf + 2, 20);
496-
memset(hash, 0, 20);
496+
px_memset(hash, 0, 20);
497497
if (res)
498498
{
499499
px_debug("mdcbuf_finish: MDC does not match");
@@ -593,7 +593,7 @@ mdcbuf_free(void *priv)
593593

594594
px_md_free(st->ctx->mdc_ctx);
595595
st->ctx->mdc_ctx = NULL;
596-
memset(st, 0, sizeof(*st));
596+
px_memset(st, 0, sizeof(*st));
597597
px_free(st);
598598
}
599599

@@ -703,7 +703,7 @@ parse_symenc_sesskey(PGP_Context *ctx, PullFilter *src)
703703
res = decrypt_key(ctx, p, res);
704704
}
705705

706-
memset(tmpbuf, 0, sizeof(tmpbuf));
706+
px_memset(tmpbuf, 0, sizeof(tmpbuf));
707707
return res;
708708
}
709709

@@ -753,6 +753,7 @@ copy_crlf(MBuf *dst, uint8 *data, int len, int *got_cr)
753753
if (res < 0)
754754
return res;
755755
}
756+
px_memset(tmpbuf, 0, sizeof(tmpbuf));
756757
return 0;
757758
}
758759

@@ -792,7 +793,7 @@ parse_literal_data(PGP_Context *ctx, MBuf *dst, PullFilter *pkt)
792793
px_debug("parse_literal_data: unexpected eof");
793794
return PXE_PGP_CORRUPT_DATA;
794795
}
795-
memset(tmpbuf, 0, 4);
796+
px_memset(tmpbuf, 0, 4);
796797

797798
/* check if text */
798799
if (ctx->text_mode)

0 commit comments

Comments
 (0)