From: Stefan Walter Date: Sun, 2 Aug 2009 20:30:24 +0000 (+0000) Subject: [egg, gcr, pkcs11] Take length of ASN.1 into account, when parsing. X-Git-Tag: split~300 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=5a17dab90d2e57b60e715ccdbb8c0e8fad38b2af;p=platform%2Fupstream%2Fgcr.git [egg, gcr, pkcs11] Take length of ASN.1 into account, when parsing. When parsing ASN.1, take length of elements into account, in order to prevent null character related vulnerabilities. --- diff --git a/egg/egg-asn1.c b/egg/egg-asn1.c index 5c78424..a1baedb 100644 --- a/egg/egg-asn1.c +++ b/egg/egg-asn1.c @@ -222,6 +222,7 @@ egg_asn1_read_value (ASN1_TYPE asn, const gchar *part, gsize *len, EggAllocator g_return_val_if_fail (asn != NULL, NULL); g_return_val_if_fail (part != NULL, NULL); + g_return_val_if_fail (len != NULL, NULL); if (allocator == NULL) allocator = (EggAllocator)g_realloc; @@ -241,7 +242,7 @@ egg_asn1_read_value (ASN1_TYPE asn, const gchar *part, gsize *len, EggAllocator if (res != ASN1_SUCCESS) { (allocator) (buf, 0); buf = NULL; - } else if (len) { + } else { *len = l; } @@ -262,20 +263,42 @@ egg_asn1_write_value (ASN1_TYPE asn, const gchar *part, return res == ASN1_SUCCESS; } +static gboolean +ascii_length_equals (const gchar *str, gconstpointer data, gsize n_data) +{ + g_assert (str); + if (!data) + return FALSE; + if (strlen (str) != n_data) + return FALSE; + return strncmp (str, data, n_data) == 0; +} + +static gboolean +ascii_length_case_equals (const gchar *str, gconstpointer data, gsize n_data) +{ + g_assert (str); + if (!data) + return FALSE; + if (strlen (str) != n_data) + return FALSE; + return g_ascii_strncasecmp (str, data, n_data) == 0; +} + gboolean egg_asn1_read_boolean (ASN1_TYPE asn, const gchar *part, gboolean *val) { gchar buffer[32]; - int n_buffer = sizeof (buffer) - 1; + int n_buffer = sizeof (buffer); int res; memset (buffer, 0, sizeof (buffer)); res = asn1_read_value (asn, part, buffer, &n_buffer); - if (res != ASN1_SUCCESS) + if (res != ASN1_SUCCESS || !n_buffer) return FALSE; - if (g_ascii_strcasecmp (buffer, "TRUE") == 0) + if (ascii_length_case_equals ("TRUE", buffer, n_buffer - 1)) *val = TRUE; else *val = FALSE; @@ -332,15 +355,20 @@ egg_asn1_read_oid (ASN1_TYPE asn, const gchar *part) { GQuark quark; guchar *buf; + gpointer end; gsize n_buf; buf = egg_asn1_read_value (asn, part, &n_buf, NULL); - if (!buf) + if (!buf || !n_buf) + return 0; + + /* Make sure the string is actually that long */ + end = memchr (buf, 0, n_buf - 1); + if (end != NULL) return 0; quark = g_quark_from_string ((gchar*)buf); g_free (buf); - return quark; } @@ -443,9 +471,9 @@ time_t timegm(struct tm *t) #endif //NOT_HAVE_TIMEGM static gboolean -parse_utc_time (const gchar *time, struct tm* when, gint *offset) +parse_utc_time (const gchar *time, gsize n_time, + struct tm* when, gint *offset) { - guint n_time; const char *p, *e; int year; @@ -453,7 +481,8 @@ parse_utc_time (const gchar *time, struct tm* when, gint *offset) g_assert (time); g_assert (offset); - n_time = strlen (time); + if (n_time != strlen (time)) + return FALSE; /* YYMMDDhhmmss.ffff Z | +0000 */ if (n_time < 6 || n_time >= 28) @@ -573,30 +602,34 @@ when_to_time (struct tm *when, gint offset) } glong -egg_asn1_time_parse_utc (const gchar *time) +egg_asn1_time_parse_utc (const gchar *time, gssize n_time) { struct tm when; gint offset; g_return_val_if_fail (time, -1); - if (!parse_utc_time (time, &when, &offset)) + if (n_time == -1) + n_time = strlen (time); + + if (!parse_utc_time (time, n_time, &when, &offset)) return -1; return when_to_time (&when, offset); } static gboolean -parse_general_time (const gchar *time, struct tm* when, gint *offset) +parse_general_time (const gchar *time, gsize n_time, + struct tm* when, gint *offset) { const char *p, *e; - guint n_time; g_assert (time); g_assert (when); g_assert (offset); - n_time = strlen (time); + if (strlen (time) != n_time) + return FALSE; /* YYYYMMDDhhmmss.ffff Z | +0000 */ if (n_time < 8 || n_time >= 30) @@ -691,14 +724,17 @@ parse_general_time (const gchar *time, struct tm* when, gint *offset) } glong -egg_asn1_time_parse_general (const gchar *time) +egg_asn1_time_parse_general (const gchar *time, gssize n_time) { struct tm when; gint offset; g_return_val_if_fail (time, -1); - if (!parse_general_time (time, &when, &offset)) + if (n_time == -1) + n_time = strlen (time); + + if (!parse_general_time (time, n_time, &when, &offset)) return -1; return when_to_time (&when, offset); @@ -716,31 +752,32 @@ read_asn1_time (ASN1_TYPE asn, const gchar *part, struct tm *when, gint *offset) g_assert (when); g_assert (offset); - len = sizeof (ttime) - 1; + len = sizeof (ttime); res = asn1_read_value (asn, part, ttime, &len); if (res != ASN1_SUCCESS) return FALSE; + --len; /* libtasn1 returns the null terminator in this count */ /* CHOICE */ - if (strcmp (ttime, "generalTime") == 0) { + if (ascii_length_equals ("generalTime", ttime, len)) { name = g_strconcat (part, ".generalTime", NULL); len = sizeof (ttime) - 1; res = asn1_read_value (asn, name, ttime, &len); g_free (name); if (res != ASN1_SUCCESS) return FALSE; - return parse_general_time (ttime, when, offset); + return parse_general_time (ttime, len - 1, when, offset); /* UTCTIME */ - } else { + } else if (ascii_length_equals ("utcTime", ttime, len)) { name = g_strconcat (part, ".utcTime", NULL); len = sizeof (ttime) - 1; res = asn1_read_value (asn, name, ttime, &len); g_free (name); if (res != ASN1_SUCCESS) return FALSE; - return parse_utc_time (ttime, when, offset); - } + return parse_utc_time (ttime, len - 1, when, offset); + } return FALSE; } @@ -807,6 +844,7 @@ dn_print_oid_value_parsed (GQuark oid, guint flags, const guchar *data, gsize le ASN1_TYPE asn1; gchar *part; gchar *value; + gsize n_value; g_assert (data); g_assert (len); @@ -824,19 +862,19 @@ dn_print_oid_value_parsed (GQuark oid, guint flags, const guchar *data, gsize le return NULL; } - value = (gchar*)egg_asn1_read_value (asn1, "", NULL, NULL); + value = (gchar*)egg_asn1_read_value (asn1, "", &n_value, NULL); /* * If it's a choice element, then we have to read depending * on what's there. */ if (value && (flags & EGG_OID_IS_CHOICE)) { - if (strcmp ("printableString", value) == 0 || - strcmp ("ia5String", value) == 0 || - strcmp ("utf8String", value) == 0 || - strcmp ("teletexString", value) == 0) { + if (ascii_length_equals ("printableString", value, n_value - 1) || + ascii_length_equals ("ia5String", value, n_value - 1 ) || + ascii_length_equals ("utf8String", value, n_value - 1) || + ascii_length_equals ("teletexString", value, n_value - 1)) { part = value; - value = (gchar*)egg_asn1_read_value (asn1, part, NULL, NULL); + value = (gchar*)egg_asn1_read_value (asn1, part, &n_value, NULL); g_free (part); } else { g_free (value); @@ -852,8 +890,8 @@ dn_print_oid_value_parsed (GQuark oid, guint flags, const guchar *data, gsize le /* * Now we make sure it's UTF-8. */ - if (!g_utf8_validate (value, -1, NULL)) { - gchar *hex = dn_print_hex_value ((guchar*)value, strlen (value)); + if (!g_utf8_validate (value, n_value, NULL)) { + gchar *hex = dn_print_hex_value ((guchar*)value, n_value); g_free (value); value = hex; } diff --git a/egg/egg-asn1.h b/egg/egg-asn1.h index dbec182..eadb05b 100644 --- a/egg/egg-asn1.h +++ b/egg/egg-asn1.h @@ -75,9 +75,9 @@ gchar* egg_asn1_read_dn (ASN1_TYPE asn, const gchar* egg_asn1_read_dn_part (ASN1_TYPE asn, const gchar *part, const gchar *match); -glong egg_asn1_time_parse_utc (const gchar* value); +glong egg_asn1_time_parse_utc (const gchar* value, gssize n_value); -glong egg_asn1_time_parse_general (const gchar* value); +glong egg_asn1_time_parse_general (const gchar* value, gssize n_value); typedef void (*EggAsn1DnCallback) (guint index, GQuark oid, const guchar *value, diff --git a/egg/tests/unit-test-asn1.c b/egg/tests/unit-test-asn1.c index 3aaec02..dfcdedb 100644 --- a/egg/tests/unit-test-asn1.c +++ b/egg/tests/unit-test-asn1.c @@ -332,7 +332,7 @@ DEFINE_TEST(general_time) const TimeTestData *data; for (data = generalized_time_test_data; data->value; ++data) { - when = egg_asn1_time_parse_general (data->value); + when = egg_asn1_time_parse_general (data->value, -1); if (data->ref != when) { printf ("%s", data->value); printf ("%s != ", ctime (&when)); @@ -350,7 +350,7 @@ DEFINE_TEST(utc_time) const TimeTestData *data; for (data = utc_time_test_data; data->value; ++data) { - when = egg_asn1_time_parse_utc (data->value); + when = egg_asn1_time_parse_utc (data->value, -1); if (data->ref != when) { printf ("%s", data->value); printf ("%s != ", ctime (&when)); diff --git a/gcr/gcr-certificate-details-widget.c b/gcr/gcr-certificate-details-widget.c index a2bdfc1..7ef7dca 100644 --- a/gcr/gcr-certificate-details-widget.c +++ b/gcr/gcr-certificate-details-widget.c @@ -182,10 +182,10 @@ append_extension (GcrCertificateDetailsWidget *self, ASN1_TYPE asn, { GQuark oid; gchar *name, *display; - gsize n_val, n_value; + gsize n_value; const guchar *value; const gchar *text; - guchar *val; + gboolean critical; int len, res; /* Make sure it is present */ @@ -225,18 +225,10 @@ append_extension (GcrCertificateDetailsWidget *self, ASN1_TYPE asn, /* Critical */ name = g_strdup_printf ("tbsCertificate.extensions.?%u.critical", index); - val = egg_asn1_read_value (asn, name, &n_val, NULL); + if (egg_asn1_read_boolean (asn, name, &critical)) + append_field_and_value (self, _("Critical"), critical ? _("Yes") : _("No"), FALSE); g_free (name); - if (!val || n_val < 1 || val[0] != 'T') - text = _("Yes"); - else - text = _("No"); - g_free (val); - - append_field_and_value (self, _("Critical"), text, FALSE); - - return TRUE; } @@ -273,7 +265,7 @@ on_parsed_dn_part (guint index, GQuark oid, const guchar *value, display = egg_asn1_dn_print_value (oid, value, n_value); if (display == NULL) - display = ""; + display = g_strdup (""); append_field_and_value (self, field, display, FALSE); g_free (field); diff --git a/gcr/gcr-certificate.c b/gcr/gcr-certificate.c index 08071cb..787394c 100644 --- a/gcr/gcr-certificate.c +++ b/gcr/gcr-certificate.c @@ -586,7 +586,7 @@ gcr_certificate_get_fingerprint_hex (GcrCertificate *self, GChecksumType type) /** * gcr_certificate_get_serial_number: * @self: a #GcrCertificate - * @length: the length of the returned data. + * @n_length: the length of the returned data. * * Get the raw binary serial number of the certificate. * @@ -601,6 +601,7 @@ gcr_certificate_get_serial_number (GcrCertificate *self, gsize *n_length) GcrCertificateInfo *info; g_return_val_if_fail (GCR_IS_CERTIFICATE (self), NULL); + g_return_val_if_fail (n_length, NULL); info = certificate_info_load (self); g_return_val_if_fail (info, NULL);