[git] GnuPG - branch, master, updated. gnupg-2.2-base-17-gb065a69
by Werner Koch
cvs at cvs.gnupg.org
Thu Aug 24 20:30:41 CEST 2017
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "The GNU Privacy Guard".
The branch, master has been updated
via b065a696344eac3007dbd5642143ecaaeebab43a (commit)
via 757302cc7a94633cd9bda5eb4dbe5c6e804b957b (commit)
from 02a5df614a369519ad7781f95dc977e24a0d4277 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit b065a696344eac3007dbd5642143ecaaeebab43a
Author: Werner Koch <wk at gnupg.org>
Date: Thu Aug 24 20:26:19 2017 +0200
gpg: Fix memory leak in sig-check.
* g10/sig-check.c (check_signature_over_key_or_uid): Remove useless
condition. Actually free when SIGNER was allocated by us.
--
SIGNER_ALLOCATED never received a value of -1 but that was tested.
IF SIGNER_ALLOCATED was 2 the memory was never freed:
if (signer_allocated == 1)
if (signer_allocated == 2)
free()
Fixes-commit: 44cdb9d73f1a0b7d2c8483a119b9c4d6caabc1ec
This function needs to be audited more thoroughly.
Signed-off-by: Werner Koch <wk at gnupg.org>
diff --git a/g10/sig-check.c b/g10/sig-check.c
index a4ef142..23af12b 100644
--- a/g10/sig-check.c
+++ b/g10/sig-check.c
@@ -878,6 +878,9 @@ check_signature_over_key_or_uid (ctrl_t ctrl, PKT_public_key *signer,
if (ret_pk)
{
signer = ret_pk;
+ /* FIXME: Using memset here is probematic because it
+ * assumes that there are no allocated fields in
+ * SIGNER. */
memset (signer, 0, sizeof (*signer));
signer_alloced = 1;
}
@@ -956,10 +959,10 @@ check_signature_over_key_or_uid (ctrl_t ctrl, PKT_public_key *signer,
gcry_md_close (md);
leave:
- if (! rc && ret_pk && (signer_alloced == -1 || ret_pk != signer))
+ if (! rc && ret_pk && ret_pk != signer)
copy_public_key (ret_pk, signer);
- if (signer_alloced == 1)
+ if (signer_alloced)
{
/* We looked up SIGNER; it is not a pointer into KB. */
release_public_key_parts (signer);
commit 757302cc7a94633cd9bda5eb4dbe5c6e804b957b
Author: Werner Koch <wk at gnupg.org>
Date: Thu Aug 24 19:19:23 2017 +0200
indent: Change comment style on two functions
--
This is to make those function better readable.
if (foo)
/* Comment */
{
}
is bad style because it requires extra time to notice the begin of the
block and vice versa when noticing the block it is not clear whether
this is an conditioned or unconditioned block.
Having asterisks on the left is better for view impaired people and
for b/w printouts.
Signed-off-by: Werner Koch <wk at gnupg.org>
diff --git a/g10/sig-check.c b/g10/sig-check.c
index 60e988e..a4ef142 100644
--- a/g10/sig-check.c
+++ b/g10/sig-check.c
@@ -738,38 +738,38 @@ check_key_signature (ctrl_t ctrl, kbnode_t root, kbnode_t node,
/* Returns whether SIGNER generated the signature SIG over the packet
- PACKET, which is a key, subkey or uid, and comes from the key block
- KB. (KB is PACKET's corresponding keyblock; we don't assume that
- SIG has been added to the keyblock.)
-
- If SIGNER is set, then checks whether SIGNER generated the
- signature. Otherwise, uses SIG->KEYID to find the alleged signer.
- This parameter can be used to effectively override the alleged
- signer that is stored in SIG.
-
- KB may be NULL if SIGNER is set.
-
- Unlike check_key_signature, this function ignores any cached
- results! That is, it does not consider SIG->FLAGS.CHECKED and
- SIG->FLAGS.VALID nor does it set them.
-
- This doesn't check the signature's semantic mean. Concretely, it
- doesn't check whether a non-self signed revocation signature was
- created by a designated revoker. In fact, it doesn't return an
- error for a binding generated by a completely different key!
-
- Returns 0 if the signature is valid. Returns GPG_ERR_SIG_CLASS if
- this signature can't be over PACKET. Returns GPG_ERR_NOT_FOUND if
- the key that generated the signature (according to SIG) could not
- be found. Returns GPG_ERR_BAD_SIGNATURE if the signature is bad.
- Other errors codes may be returned if something else goes wrong.
-
- IF IS_SELFSIG is not NULL, sets *IS_SELFSIG to 1 if this is a
- self-signature (by the key's primary key) or 0 if not.
-
- If RET_PK is not NULL, returns a copy of the public key that
- generated the signature (i.e., the signer) on success. This must
- be released by the caller using release_public_key_parts (). */
+ * PACKET, which is a key, subkey or uid, and comes from the key block
+ * KB. (KB is PACKET's corresponding keyblock; we don't assume that
+ * SIG has been added to the keyblock.)
+ *
+ * If SIGNER is set, then checks whether SIGNER generated the
+ * signature. Otherwise, uses SIG->KEYID to find the alleged signer.
+ * This parameter can be used to effectively override the alleged
+ * signer that is stored in SIG.
+ *
+ * KB may be NULL if SIGNER is set.
+ *
+ * Unlike check_key_signature, this function ignores any cached
+ * results! That is, it does not consider SIG->FLAGS.CHECKED and
+ * SIG->FLAGS.VALID nor does it set them.
+ *
+ * This doesn't check the signature's semantic mean. Concretely, it
+ * doesn't check whether a non-self signed revocation signature was
+ * created by a designated revoker. In fact, it doesn't return an
+ * error for a binding generated by a completely different key!
+ *
+ * Returns 0 if the signature is valid. Returns GPG_ERR_SIG_CLASS if
+ * this signature can't be over PACKET. Returns GPG_ERR_NOT_FOUND if
+ * the key that generated the signature (according to SIG) could not
+ * be found. Returns GPG_ERR_BAD_SIGNATURE if the signature is bad.
+ * Other errors codes may be returned if something else goes wrong.
+ *
+ * IF IS_SELFSIG is not NULL, sets *IS_SELFSIG to 1 if this is a
+ * self-signature (by the key's primary key) or 0 if not.
+ *
+ * If RET_PK is not NULL, returns a copy of the public key that
+ * generated the signature (i.e., the signer) on success. This must
+ * be released by the caller using release_public_key_parts (). */
gpg_error_t
check_signature_over_key_or_uid (ctrl_t ctrl, PKT_public_key *signer,
PKT_signature *sig, KBNODE kb, PACKET *packet,
@@ -796,8 +796,8 @@ check_signature_over_key_or_uid (ctrl_t ctrl, PKT_public_key *signer,
/* Primary key revocation. */
|| sig->sig_class == 0x20)
{
+ /* Key revocations can only be over primary keys. */
if (packet->pkttype != PKT_PUBLIC_KEY)
- /* Key revocations can only be over primary keys. */
return gpg_error (GPG_ERR_SIG_CLASS);
}
else if (/* Subkey binding. */
@@ -840,19 +840,19 @@ check_signature_over_key_or_uid (ctrl_t ctrl, PKT_public_key *signer,
/* Get the signer. If possible, avoid a look up. */
if (sig->keyid[0] == pripk->keyid[0]
&& sig->keyid[1] == pripk->keyid[1])
- /* Issued by the primary key. */
{
+ /* Issued by the primary key. */
signer = pripk;
if (is_selfsig)
*is_selfsig = 1;
}
else
{
+ /* See if one of the subkeys was the signer (although this
+ is extremely unlikely). */
kbnode_t ctx = NULL;
kbnode_t n;
- /* See if one of the subkeys was the signer (although this
- is extremely unlikely). */
while ((n = walk_kbnode (kb, &ctx, 0)))
{
PKT_public_key *subk;
@@ -863,16 +863,16 @@ check_signature_over_key_or_uid (ctrl_t ctrl, PKT_public_key *signer,
subk = n->pkt->pkt.public_key;
if (sig->keyid[0] == subk->keyid[0]
&& sig->keyid[1] == subk->keyid[1])
- /* Issued by a subkey. */
{
+ /* Issued by a subkey. */
signer = subk;
break;
}
}
if (! signer)
- /* Signer by some other key. */
{
+ /* Signer by some other key. */
if (is_selfsig)
*is_selfsig = 0;
if (ret_pk)
@@ -893,14 +893,14 @@ check_signature_over_key_or_uid (ctrl_t ctrl, PKT_public_key *signer,
xfree (signer);
signer = NULL;
signer_alloced = 0;
- goto out;
+ goto leave;
}
}
}
}
/* We checked above that we supported this algo, so an error here is
- a bug. */
+ * a bug. */
if (gcry_md_open (&md, sig->digest_algo, 0))
BUG ();
@@ -916,7 +916,7 @@ check_signature_over_key_or_uid (ctrl_t ctrl, PKT_public_key *signer,
rc = check_signature_end_simple (signer, sig, md);
}
else if (/* Primary key binding (made by a subkey). */
- sig->sig_class == 0x19)
+ sig->sig_class == 0x19)
{
log_assert (packet->pkttype == PKT_PUBLIC_KEY);
hash_public_key (md, packet->pkt.public_key);
@@ -947,27 +947,31 @@ check_signature_over_key_or_uid (ctrl_t ctrl, PKT_public_key *signer,
rc = check_signature_end_simple (signer, sig, md);
}
else
- /* We should never get here. (The first if above should have
- already caught this error.) */
- BUG ();
+ {
+ /* We should never get here. (The first if above should have
+ * already caught this error.) */
+ BUG ();
+ }
gcry_md_close (md);
- out:
+ leave:
if (! rc && ret_pk && (signer_alloced == -1 || ret_pk != signer))
copy_public_key (ret_pk, signer);
+
if (signer_alloced == 1)
- /* We looked up SIGNER; it is not a pointer into KB. */
{
+ /* We looked up SIGNER; it is not a pointer into KB. */
release_public_key_parts (signer);
+ /* Free if we also allocated the memory. */
if (signer_alloced == 2)
- /* We also allocated the memory. */
xfree (signer);
}
return rc;
}
+
/* Check that a signature over a key (e.g., a key revocation, key
* binding, user id certification, etc.) is valid. If the function
* detects a self-signature, it uses the public key from the specified
@@ -1027,9 +1031,9 @@ check_key_signature2 (ctrl_t ctrl,
algo = sig->digest_algo;
/* Check whether we have cached the result of a previous signature
- check. Note that we may no longer have the pubkey or hash
- needed to verify a sig, but can still use the cached value. A
- cache refresh detects and clears these cases. */
+ * check. Note that we may no longer have the pubkey or hash
+ * needed to verify a sig, but can still use the cached value. A
+ * cache refresh detects and clears these cases. */
if ( !opt.no_sig_cache )
{
cache_stats.total++;
@@ -1045,7 +1049,7 @@ check_key_signature2 (ctrl_t ctrl,
*is_selfsig = 1;
}
/* BUG: This is wrong for non-self-sigs... needs to be the
- actual pk. */
+ * actual pk. */
rc = check_signature_metadata_validity (pk, sig, r_expired, NULL);
if (rc)
return rc;
@@ -1094,13 +1098,15 @@ check_key_signature2 (ctrl_t ctrl,
rc = check_signature_metadata_validity (pk, sig,
r_expired, NULL);
if (! rc)
- /* 0x28 must be a self-sig, but 0x18 needn't be. */
- rc = check_signature_over_key_or_uid (ctrl,
- sig->sig_class == 0x18
- ? NULL : pk,
- sig, root, snode->pkt,
- is_selfsig, ret_pk);
- }
+ {
+ /* 0x28 must be a self-sig, but 0x18 needn't be. */
+ rc = check_signature_over_key_or_uid (ctrl,
+ sig->sig_class == 0x18
+ ? NULL : pk,
+ sig, root, snode->pkt,
+ is_selfsig, ret_pk);
+ }
+ }
else
{
if (opt.verbose)
@@ -1137,11 +1143,13 @@ check_key_signature2 (ctrl_t ctrl,
{
rc = check_signature_metadata_validity (pk, sig, r_expired, NULL);
if (! rc)
- /* If this is a self-sig, ignore check_pk. */
- rc = check_signature_over_key_or_uid
- (ctrl,
- keyid_cmp (pk_keyid (pk), sig->keyid) == 0 ? pk : check_pk,
- sig, root, unode->pkt, NULL, ret_pk);
+ {
+ /* If this is a self-sig, ignore check_pk. */
+ rc = check_signature_over_key_or_uid
+ (ctrl,
+ keyid_cmp (pk_keyid (pk), sig->keyid) == 0 ? pk : check_pk,
+ sig, root, unode->pkt, NULL, ret_pk);
+ }
}
else
{
-----------------------------------------------------------------------
Summary of changes:
g10/sig-check.c | 141 ++++++++++++++++++++++++++++++--------------------------
1 file changed, 76 insertions(+), 65 deletions(-)
hooks/post-receive
--
The GNU Privacy Guard
http://git.gnupg.org
More information about the Gnupg-commits
mailing list