From: Jim Castleberry <jcastle@in-system.com>
Date: Wed, 1 Nov 1995 03:37:13 +0800
To: cypherpunks@toad.com
Subject: PGP 2.6.2 signator replacement bug fix
Message-ID: <9510311803.AA11158@toad.com>
MIME-Version: 1.0
Content-Type: text/plain


-----BEGIN PGP SIGNED MESSAGE-----


Here's a fix for a minor bug in PGP 2.6.2.  I reported it to pgp-bugs last
February and they responded promptly, but it hasn't been propagated to the
release - I suppose 2.6.2 is "in the freezer".

Anyway, the bug can cause a crash and other folks might like to put in the
fix, so here it is.  Anyone have any others?

Jim Castleberry
jcastle@in-system.com

Original bug report starts here:
- -------------------------------------------------------------------------

I believe I've identified a bug in PGP 2.6.2.  The bug occurs on my system:
	HP 9000/735, running HPUX 9.03
	MIT PGP 2.6.2
	Compiled with either the native cc or gcc 2.6.3
	Optimization level doesn't change the behavior (from -O0 to +O3).
	pubring.pgp contains all the keys on the servers, plus a few
	   unpublished keys and some additional signatures.

When an existing signature is replaced by a newer one by the same signator,
memory gets corrupted.  On my system, this results in a program abort.

The problem is in file keyadd.c, function mergesigs().  When a signature is
(possibly) going to be replaced, user_from_keyID() is called to get a pointer
to the signator's user ID (as a C string in the in-memory user hash table.
Then that pointer is passed to check_key_sig(), which passes it to
getpublickey(), which passes it to readpublickey(), which reads successive
user ID strings into it while it searches for the desired keyID.  But the hash
table contains dynamically allocated strings that are only as long as the
string they hold.  So when a longer user ID is read into it, memory past the
end of the C string is corrupted.  That trashes a hash table entry ("struct
hashent"), destroying pointers and resulting in an illegal memory access later
in the program.

The offending code in mergesigs() only gets executed when a signature is being
replaced with a more recent one by the same signator.  It only showed up
because I updated my pubring with all new keys on the servers for the last 31
days, and one key has an updated sig on it:
   pub  1024/AB1F4831 1993/05/10 Robert Walking-Owl <rrothenb@ic.sunysb.edu>
			         Robert Walking-Owl <robert.rothenburg@asb.com>

Here's a context diff for the fix.  The signator userid isn't needed by
check_key_sig() or the functions it calls since the signator's keyID is given,
and if the userid is NULL then readkeypacket() ignores it, so just pass NULL to
check_key_sig() and use the signator C string in mergesigs() as-is.

Jim Castleberry

- - cut here -------------------------------------------------------------------

*** old/keyadd.c	Wed Oct  5 20:48:11 1994
- --- keyadd.c	Wed Feb 15 22:37:53 1995
***************
*** 196,206 ****
  			    status = check_key_sig(fring,
  						   KeyIDpos, KeyIDlen,
  						   userid, fkey, keypos,
! 						   ringfile, signator,
  						   (byte *) & xstamp,
  						   &sigClass);
  			PascalToC(userid);
- - 			PascalToC(signator);
  			if (!status) {
  			    fprintf(pgpout,
  				    LANG("Replacing signature from %s\n"),
- --- 196,205 ----
  			    status = check_key_sig(fring,
  						   KeyIDpos, KeyIDlen,
  						   userid, fkey, keypos,
! 						   ringfile, NULL,
  						   (byte *) & xstamp,
  						   &sigClass);
  			PascalToC(userid);
  			if (!status) {
  			    fprintf(pgpout,
  				    LANG("Replacing signature from %s\n"),

-----BEGIN PGP SIGNATURE-----
Version: 2.6.2

iQEVAwUBMJZkDJYkAuN55UFhAQG8aQf/bsx7xlS9P9SBE8ysMEbjMkKwdf6yZSCt
fd0xBsUbTIsIS2Gzlx0ux7N3HWqJm/og2EBqebhfEzQk9IUBK8sp8Kbv8GtbUWrj
ai2b6/hxu2yXbtQEAmTDJLO3fQCZ6yjkydFYaz2n+HuRxN4h6TlWwIHzQWOBCxuK
qzPENr+c4YgMCHMVLGnAHDgPA8G9J/xX3UIPKQJQnrpAJwlGf+LTOkWeA+/kMLiw
aos108n6698mcsIxXzHltCVlCaYYY6meFMHzD4rGjXOCURTtKTSRx1dJ7WNSn+1N
JNwJSnFXrsvJRodko0fjpYnCUDNEbmEQgDsJX80wTdDfI+JH2LM/+g==
=jh+s
-----END PGP SIGNATURE-----