egg-armor: Fix memrchr() call with negative string length

 * Initial patch by Gustavo Luiz Duarte <gustavold@linux.vnet.ibm.com>
 * The cause of this bug was reusing argument variables for other
   purposes in parsing functions when that didn't really make sense,
   so fix this as well.
 * Add tests that catch this issue.

See https://bugzilla.redhat.com/show_bug.cgi?id=893162

https://bugzilla.gnome.org/show_bug.cgi?id=691500

3 files changed, 173 insertions, 13 deletions

diff --git a/egg/egg-armor.c b/egg/egg-armor.c
index ec514cc2..7c788ade 100644
--- a/egg/egg-armor.c
+++ b/egg/egg-armor.c
@@ -104,18 +104,20 @@ armor_find_begin (const gchar *data,
                   const gchar **outer)
 {
 	const gchar *pref, *suff;
+	const gchar *at;
 	gchar *stype;
+	gsize len;
 
 	/* Look for a prefix */
 	pref = g_strstr_len ((gchar*)data, n_data, ARMOR_PREF_BEGIN);
 	if (!pref)
 		return NULL;
 
-	n_data -= (pref - data) + ARMOR_PREF_BEGIN_L;
-	data = pref + ARMOR_PREF_BEGIN_L;
+	len = n_data - ((pref - data) + ARMOR_PREF_BEGIN_L);
+	at = pref + ARMOR_PREF_BEGIN_L;
 
 	/* Look for the end of that begin */
-	suff = g_strstr_len ((gchar*)data, n_data, ARMOR_SUFF);
+	suff = g_strstr_len ((gchar *)at, len, ARMOR_SUFF);
 	if (!suff)
 		return NULL;
 
@@ -149,6 +151,8 @@ armor_find_end (const gchar *data,
 	const gchar *stype;
 	const gchar *pref;
 	const gchar *line;
+	const gchar *at;
+	gsize len;
 	gsize n_type;
 
 	/* Look for a prefix */
@@ -156,20 +160,20 @@ armor_find_end (const gchar *data,
 	if (!pref)
 		return NULL;
 
-	n_data -= (pref - data) + ARMOR_PREF_END_L;
-	data = pref + ARMOR_PREF_END_L;
+	len = n_data - ((pref - data) + ARMOR_PREF_END_L);
+	at = pref + ARMOR_PREF_END_L;
 
 	/* Next comes the type string */
 	stype = g_quark_to_string (type);
 	n_type = strlen (stype);
-	if (n_type > n_data || strncmp ((gchar*)data, stype, n_type) != 0)
+	if (n_type > len || strncmp ((gchar*)at, stype, n_type) != 0)
 		return NULL;
 
-	n_data -= n_type;
-	data += n_type;
+	len -= n_type;
+	at += n_type;
 
 	/* Next comes the suffix */
-	if (ARMOR_SUFF_L > n_data && strncmp ((gchar*)data, ARMOR_SUFF, ARMOR_SUFF_L) != 0)
+	if (ARMOR_SUFF_L > len && strncmp ((gchar*)at, ARMOR_SUFF, ARMOR_SUFF_L) != 0)
 		return NULL;
 
 	/*
@@ -182,10 +186,10 @@ armor_find_end (const gchar *data,
 		pref = line;
 
 	if (outer != NULL) {
-		data += ARMOR_SUFF_L;
-		if (isspace (data[0]))
-			data++;
-		*outer = data;
+		at += ARMOR_SUFF_L;
+		if (isspace (at[0]))
+			at++;
+		*outer = at;
 	}
 
 	/* The end of the data */
diff --git a/egg/tests/Makefile.am b/egg/tests/Makefile.am
index 669e8964..b523e88e 100644
--- a/egg/tests/Makefile.am
+++ b/egg/tests/Makefile.am
@@ -31,6 +31,7 @@ TEST_PROGS = \
 	test-secmem \
 	test-padding \
 	test-symkey \
+	test-armor \
 	test-openssl \
 	test-dh \
 	test-spawn
diff --git a/egg/tests/test-armor.c b/egg/tests/test-armor.c
new file mode 100644
index 00000000..d5a366b7
--- /dev/null
+++ b/egg/tests/test-armor.c
@@ -0,0 +1,155 @@
+/* -*- Mode: C; indent-tabs-mode: t; c-basic-offset: 8; tab-width: 8 -*- */
+/* test-armor.c: Test PEM and Armor parsing
+
+   Copyright (C) 2012 Red Hat Inc.
+
+   The Gnome Keyring Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Library General Public License as
+   published by the Free Software Foundation; either version 2 of the
+   License, or (at your option) any later version.
+
+   The Gnome Keyring Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Library General Public License for more details.
+
+   You should have received a copy of the GNU Library General Public
+   License along with the Gnome Library; see the file COPYING.LIB.  If not,
+   write to the Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+   Boston, MA 02111-1307, USA.
+
+   Author: Stef Walter <stefw@gnome.org>
+*/
+
+#include "config.h"
+
+#include "egg/egg-armor.h"
+#include "egg/egg-symkey.h"
+#include "egg/egg-openssl.h"
+#include "egg/egg-secure-memory.h"
+#include "egg/egg-testing.h"
+
+#include <glib.h>
+
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include <unistd.h>
+
+EGG_SECURE_DEFINE_GLIB_GLOBALS ();
+
+static void
+on_pem_get_contents (GQuark type,
+                     GBytes *data,
+                     GBytes *outer,
+                     GHashTable *headers,
+                     gpointer user_data)
+{
+	GBytes **contents = user_data;
+
+	g_assert_cmpstr (g_quark_to_string (type), ==, "TEST");
+	g_assert (*contents == NULL);
+	*contents = g_bytes_ref (data);
+}
+
+
+static void
+test_armor_parse (void)
+{
+	const char *pem_data = "-----BEGIN TEST-----\n"
+	                       "Z29vZCBtb3JuaW5nIGV2ZXJ5b25lCg==\n"
+	                       "-----END TEST-----\n";
+
+	GBytes *contents = NULL;
+	GBytes *check;
+	GBytes *bytes;
+	guint num;
+
+	bytes = g_bytes_new_static (pem_data, strlen (pem_data));
+
+	num = egg_armor_parse (bytes, on_pem_get_contents, &contents);
+	g_assert_cmpint (num, ==, 1);
+	g_assert (contents != NULL);
+
+	check = g_bytes_new ("good morning everyone\n", 22);
+	g_assert (g_bytes_equal (check, contents));
+
+	g_bytes_unref (check);
+	g_bytes_unref (contents);
+	g_bytes_unref (bytes);
+}
+
+static void
+test_armor_skip_checksum (void)
+{
+	const char *pem_data = "-----BEGIN TEST-----\n"
+	                       "Z29vZCBtb3JuaW5nIGV2ZXJ5b25lCg==\n"
+	                       "=checksum"
+	                       "-----END TEST-----\n";
+
+	GBytes *contents = NULL;
+	GBytes *check;
+	GBytes *bytes;
+	guint num;
+
+	/* Check that the (above invalid) OpenPGP checksum is skipped */
+
+	bytes = g_bytes_new_static (pem_data, strlen (pem_data));
+
+	num = egg_armor_parse (bytes, on_pem_get_contents, &contents);
+	g_assert_cmpint (num, ==, 1);
+	g_assert (contents != NULL);
+
+	check = g_bytes_new ("good morning everyone\n", 22);
+	g_assert (g_bytes_equal (check, contents));
+
+	g_bytes_unref (check);
+	g_bytes_unref (contents);
+	g_bytes_unref (bytes);
+}
+
+static void
+test_invalid (gconstpointer data)
+{
+	GBytes *bytes;
+	guint num;
+
+	/* Invalid opening line above */
+
+	bytes = g_bytes_new_static (data, strlen (data));
+
+	num = egg_armor_parse (bytes, NULL, NULL);
+	g_assert_cmpint (num, ==, 0);
+
+	g_bytes_unref (bytes);
+}
+
+int
+main (int argc, char **argv)
+{
+	g_test_init (&argc, &argv, NULL);
+
+	g_test_add_func ("/armor/parse", test_armor_parse);
+	g_test_add_func ("/armor/skip-checksum", test_armor_skip_checksum);
+
+	g_test_add_data_func ("/armor/invalid-start",
+	                      "-----BEGIN TEST--",
+	                      test_invalid);
+	g_test_add_data_func ("/armor/invalid-end",
+	                      "-----BEGIN TEST-----\n"
+	                      "Z29vZCBtb3JuaW5nIGV2ZXJ5b25lCg==\n"
+	                      "--END TEST-----\n",
+	                      test_invalid);
+	g_test_add_data_func ("/armor/invalid-mismatch",
+	                      "-----BEGIN TEST-----\n"
+	                      "Z29vZCBtb3JuaW5nIGV2ZXJ5b25lCg==\n"
+	                      "-----END CERTIFICATE-----\n",
+	                      test_invalid);
+	g_test_add_data_func ("/armor/invalid-suffix",
+	                      "-----BEGIN TEST-----\n"
+	                      "Z29vZCBtb3JuaW5nIGV2ZXJ5b25lCg==\n"
+	                      "-----END TEST--\n",
+	                      test_invalid);
+
+	return g_test_run ();
+}