Bug 693503: Fix SEGV/memory problems in AES.

If an illegal keysize is passed into the AES crypt filter, we
currently exit without setting up the AES context. This causes
us to fail in all manner of ways later on.

We now return failure and callers throw an exception.

This appears to solve all the SEGVs and memory exceptions found in
crypt_aes by Mateusz "j00ru" Jurczyk and Gynvael Coldwind of the
Google Security Team using Address Sanitizer. Many thanks!

4 files changed, 37 insertions, 28 deletions

diff --git a/fitz/crypt_aes.c b/fitz/crypt_aes.c
index 4d8c4498..27db64fc 100644
--- a/fitz/crypt_aes.c
+++ b/fitz/crypt_aes.c
@@ -176,7 +176,7 @@ static void aes_gen_tables( void )
 /*
  * AES key schedule (encryption)
  */
-void aes_setkey_enc( aes_context *ctx, const unsigned char *key, int keysize )
+int aes_setkey_enc( aes_context *ctx, const unsigned char *key, int keysize )
 {
 	int i;
 	unsigned long *RK;
@@ -194,7 +194,7 @@ void aes_setkey_enc( aes_context *ctx, const unsigned char *key, int keysize )
 	case 128: ctx->nr = 10; break;
 	case 192: ctx->nr = 12; break;
 	case 256: ctx->nr = 14; break;
-	default : return;
+	default : return 1;
 	}
 
 #if defined(PADLOCK_ALIGN16)
@@ -274,12 +274,13 @@ void aes_setkey_enc( aes_context *ctx, const unsigned char *key, int keysize )
 
 		break;
 	}
+	return 0;
 }
 
 /*
  * AES key schedule (decryption)
  */
-void aes_setkey_dec( aes_context *ctx, const unsigned char *key, int keysize )
+int aes_setkey_dec(aes_context *ctx, const unsigned char *key, int keysize)
 {
 	int i, j;
 	aes_context cty;
@@ -291,7 +292,7 @@ void aes_setkey_dec( aes_context *ctx, const unsigned char *key, int keysize )
 	case 128: ctx->nr = 10; break;
 	case 192: ctx->nr = 12; break;
 	case 256: ctx->nr = 14; break;
-	default : return;
+	default: return 1;
 	}
 
 #if defined(PADLOCK_ALIGN16)
@@ -300,7 +301,9 @@ void aes_setkey_dec( aes_context *ctx, const unsigned char *key, int keysize )
 	ctx->rk = RK = ctx->buf;
 #endif
 
-	aes_setkey_enc( &cty, key, keysize );
+	i = aes_setkey_enc( &cty, key, keysize );
+	if (i)
+		return i;
 	SK = cty.rk + cty.nr * 4;
 
 	*RK++ = *SK++;
@@ -325,6 +328,7 @@ void aes_setkey_dec( aes_context *ctx, const unsigned char *key, int keysize )
 	*RK++ = *SK++;
 
 	memset( &cty, 0, sizeof( aes_context ) );
+	return 0;
 }
 
 #define AES_FROUND(X0,X1,X2,X3,Y0,Y1,Y2,Y3)	\
diff --git a/fitz/filt_basic.c b/fitz/filt_basic.c
index db95533d..6f07edd6 100644
--- a/fitz/filt_basic.c
+++ b/fitz/filt_basic.c
@@ -653,7 +653,8 @@ fz_open_aesd(fz_stream *chain, unsigned char *key, unsigned keylen)
 	{
 		state = fz_malloc_struct(ctx, fz_aesd);
 		state->chain = chain;
-		aes_setkey_dec(&state->aes, key, keylen * 8);
+		if (aes_setkey_dec(&state->aes, key, keylen * 8))
+			fz_throw(ctx, "AES key init failed (keylen=%d)", keylen * 8);
 		state->ivcount = 0;
 		state->rp = state->bp;
 		state->wp = state->bp;
diff --git a/fitz/fitz-internal.h b/fitz/fitz-internal.h
index c90474dd..0de2e4c2 100644
--- a/fitz/fitz-internal.h
+++ b/fitz/fitz-internal.h
@@ -393,8 +393,8 @@ struct fz_aes_s
 	unsigned long buf[68]; /* unaligned data */
 };
 
-void aes_setkey_enc( fz_aes *ctx, const unsigned char *key, int keysize );
-void aes_setkey_dec( fz_aes *ctx, const unsigned char *key, int keysize );
+int aes_setkey_enc( fz_aes *ctx, const unsigned char *key, int keysize );
+int aes_setkey_dec( fz_aes *ctx, const unsigned char *key, int keysize );
 void aes_crypt_cbc( fz_aes *ctx, int mode, int length,
 	unsigned char iv[16],
 	const unsigned char *input,
diff --git a/pdf/pdf_crypt.c b/pdf/pdf_crypt.c
index ec6f792b..916629f2 100644
--- a/pdf/pdf_crypt.c
+++ b/pdf/pdf_crypt.c
@@ -403,7 +403,7 @@ pdf_compute_encryption_key(pdf_crypt *crypt, unsigned char *password, int pwlen,
  */
 
 static void
-pdf_compute_encryption_key_r5(pdf_crypt *crypt, unsigned char *password, int pwlen, int ownerkey, unsigned char *validationkey)
+pdf_compute_encryption_key_r5(fz_context *ctx, pdf_crypt *crypt, unsigned char *password, int pwlen, int ownerkey, unsigned char *validationkey)
 {
 	unsigned char buffer[128 + 8 + 48];
 	fz_sha256 sha256;
@@ -439,7 +439,8 @@ pdf_compute_encryption_key_r5(pdf_crypt *crypt, unsigned char *password, int pwl
 
 	/* clear password buffer and use it as iv */
 	memset(buffer + 32, 0, sizeof(buffer) - 32);
-	aes_setkey_dec(&aes, buffer, crypt->length);
+	if (aes_setkey_dec(&aes, buffer, crypt->length))
+		fz_throw(ctx, "AES key init failed (keylen=%d)", crypt->length);
 	aes_crypt_cbc(&aes, AES_DECRYPT, 32, buffer + 32, ownerkey ? crypt->oe : crypt->ue, crypt->key);
 }
 
@@ -451,7 +452,7 @@ pdf_compute_encryption_key_r5(pdf_crypt *crypt, unsigned char *password, int pwl
  */
 
 static void
-pdf_compute_hardened_hash_r6(unsigned char *password, int pwlen, unsigned char salt[16], unsigned char *ownerkey, unsigned char hash[32])
+pdf_compute_hardened_hash_r6(fz_context *ctx, unsigned char *password, int pwlen, unsigned char salt[16], unsigned char *ownerkey, unsigned char hash[32])
 {
 	unsigned char data[(128 + 64 + 48) * 64];
 	unsigned char block[64];
@@ -483,7 +484,8 @@ pdf_compute_hardened_hash_r6(unsigned char *password, int pwlen, unsigned char s
 			memcpy(data + j * data_len, data, data_len);
 
 		/* Step 3: encrypt data using data block as key and iv */
-		aes_setkey_enc(&aes, block, 128);
+		if (aes_setkey_enc(&aes, block, 128))
+			fz_throw(ctx, "AES key init failed (keylen=%d)", 128);
 		aes_crypt_cbc(&aes, AES_ENCRYPT, data_len * 64, block + 16, data, data);
 
 		/* Step 4: determine SHA-2 hash size for this round */
@@ -517,7 +519,7 @@ pdf_compute_hardened_hash_r6(unsigned char *password, int pwlen, unsigned char s
 }
 
 static void
-pdf_compute_encryption_key_r6(pdf_crypt *crypt, unsigned char *password, int pwlen, int ownerkey, unsigned char *validationkey)
+pdf_compute_encryption_key_r6(fz_context *ctx, pdf_crypt *crypt, unsigned char *password, int pwlen, int ownerkey, unsigned char *validationkey)
 {
 	unsigned char hash[32];
 	unsigned char iv[16];
@@ -526,14 +528,15 @@ pdf_compute_encryption_key_r6(pdf_crypt *crypt, unsigned char *password, int pwl
 	if (pwlen > 127)
 		pwlen = 127;
 
-	pdf_compute_hardened_hash_r6(password, pwlen,
+	pdf_compute_hardened_hash_r6(ctx, password, pwlen,
 		(ownerkey ? crypt->o : crypt->u) + 32,
 		ownerkey ? crypt->u : NULL, validationkey);
-	pdf_compute_hardened_hash_r6(password, pwlen,
+	pdf_compute_hardened_hash_r6(ctx, password, pwlen,
 		crypt->u + 40, NULL, hash);
 
 	memset(iv, 0, sizeof(iv));
-	aes_setkey_dec(&aes, hash, 256);
+	if (aes_setkey_dec(&aes, hash, 256))
+		fz_throw(ctx, "AES key init failed (keylen=256)");
 	aes_crypt_cbc(&aes, AES_DECRYPT, 32, iv,
 		ownerkey ? crypt->oe : crypt->ue, crypt->key);
 }
@@ -544,7 +547,7 @@ pdf_compute_encryption_key_r6(pdf_crypt *crypt, unsigned char *password, int pwl
  */
 
 static void
-pdf_compute_user_password(pdf_crypt *crypt, unsigned char *password, int pwlen, unsigned char *output)
+pdf_compute_user_password(fz_context *ctx, pdf_crypt *crypt, unsigned char *password, int pwlen, unsigned char *output)
 {
 	if (crypt->r == 2)
 	{
@@ -588,12 +591,12 @@ pdf_compute_user_password(pdf_crypt *crypt, unsigned char *password, int pwlen,
 
 	if (crypt->r == 5)
 	{
-		pdf_compute_encryption_key_r5(crypt, password, pwlen, 0, output);
+		pdf_compute_encryption_key_r5(ctx, crypt, password, pwlen, 0, output);
 	}
 
 	if (crypt->r == 6)
 	{
-		pdf_compute_encryption_key_r6(crypt, password, pwlen, 0, output);
+		pdf_compute_encryption_key_r6(ctx, crypt, password, pwlen, 0, output);
 	}
 }
 
@@ -605,10 +608,10 @@ pdf_compute_user_password(pdf_crypt *crypt, unsigned char *password, int pwlen,
  */
 
 static int
-pdf_authenticate_user_password(pdf_crypt *crypt, unsigned char *password, int pwlen)
+pdf_authenticate_user_password(fz_context *ctx, pdf_crypt *crypt, unsigned char *password, int pwlen)
 {
 	unsigned char output[32];
-	pdf_compute_user_password(crypt, password, pwlen, output);
+	pdf_compute_user_password(ctx, crypt, password, pwlen, output);
 	if (crypt->r == 2 || crypt->r == 5 || crypt->r == 6)
 		return memcmp(output, crypt->u, 32) == 0;
 	if (crypt->r == 3 || crypt->r == 4)
@@ -624,7 +627,7 @@ pdf_authenticate_user_password(pdf_crypt *crypt, unsigned char *password, int pw
  */
 
 static int
-pdf_authenticate_owner_password(pdf_crypt *crypt, unsigned char *ownerpass, int pwlen)
+pdf_authenticate_owner_password(fz_context *ctx, pdf_crypt *crypt, unsigned char *ownerpass, int pwlen)
 {
 	unsigned char pwbuf[32];
 	unsigned char key[32];
@@ -637,13 +640,13 @@ pdf_authenticate_owner_password(pdf_crypt *crypt, unsigned char *ownerpass, int
 	if (crypt->r == 5)
 	{
 		/* PDF 1.7 ExtensionLevel 3 algorithm 3.12 */
-		pdf_compute_encryption_key_r5(crypt, ownerpass, pwlen, 1, key);
+		pdf_compute_encryption_key_r5(ctx, crypt, ownerpass, pwlen, 1, key);
 		return !memcmp(key, crypt->o, 32);
 	}
 	else if (crypt->r == 6)
 	{
 		/* PDF 1.7 ExtensionLevel 8 algorithm */
-		pdf_compute_encryption_key_r6(crypt, ownerpass, pwlen, 1, key);
+		pdf_compute_encryption_key_r6(ctx, crypt, ownerpass, pwlen, 1, key);
 		return !memcmp(key, crypt->o, 32);
 	}
 
@@ -693,7 +696,7 @@ pdf_authenticate_owner_password(pdf_crypt *crypt, unsigned char *ownerpass, int
 		}
 	}
 
-	return pdf_authenticate_user_password(crypt, userpass, 32);
+	return pdf_authenticate_user_password(ctx, crypt, userpass, 32);
 }
 
 int
@@ -703,9 +706,9 @@ pdf_authenticate_password(pdf_document *xref, char *password)
 	{
 		if (!password)
 			password = "";
-		if (pdf_authenticate_user_password(xref->crypt, (unsigned char *)password, strlen(password)))
+		if (pdf_authenticate_user_password(xref->ctx, xref->crypt, (unsigned char *)password, strlen(password)))
 			return 1;
-		if (pdf_authenticate_owner_password(xref->crypt, (unsigned char *)password, strlen(password)))
+		if (pdf_authenticate_owner_password(xref->ctx, xref->crypt, (unsigned char *)password, strlen(password)))
 			return 1;
 		return 0;
 	}
@@ -860,7 +863,8 @@ pdf_crypt_obj_imp(fz_context *ctx, pdf_crypt *crypt, pdf_obj *obj, unsigned char
 				unsigned char iv[16];
 				fz_aes aes;
 				memcpy(iv, s, 16);
-				aes_setkey_dec(&aes, key, keylen * 8);
+				if (aes_setkey_dec(&aes, key, keylen * 8))
+					fz_throw(ctx, "AES key init failed (keylen=%d)", keylen * 8);
 				aes_crypt_cbc(&aes, AES_DECRYPT, n - 16, iv, s + 16, s);
 				/* delete space used for iv and padding bytes at end */
 				if (s[n - 17] < 1 || s[n - 17] > 16)