From 155912092ab90b5597817b4a3c67ccc8dec00cb9 Mon Sep 17 00:00:00 2001 From: Stef Walter Date: Fri, 11 Jan 2013 20:23:12 +0000 Subject: egg-armor: Fix memrchr() call with negative string length * Initial patch by Gustavo Luiz Duarte * 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=691505 --- diff --git a/egg/egg-armor.c b/egg/egg-armor.c index 812f1aa..dd213d0 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 2e8335b..7cb1830 100644 --- a/egg/tests/Makefile.am +++ b/egg/tests/Makefile.am @@ -32,6 +32,7 @@ TEST_PROGS = \ test-secmem \ test-padding \ test-symkey \ + test-armor \ test-openssl \ test-dh diff --git a/egg/tests/test-armor.c b/egg/tests/test-armor.c new file mode 100644 index 0000000..d5a366b --- a/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 +*/ + +#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 + +#include +#include +#include +#include + +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 (); +} -- cgit v0.9.0.2