Bug 62479 - segfault when parsing the anchors
Summary: segfault when parsing the anchors
Status: RESOLVED FIXED
Alias: None
Product: p11-glue
Classification: Unclassified
Component: p11-kit (show other bugs)
Version: unspecified
Hardware: All OpenBSD
: medium major
Assignee: Stef Walter
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-03-18 16:44 UTC by Antoine Jacoutot
Modified: 2013-03-19 15:47 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
add missing include for libgen.h (242 bytes, patch)
2013-03-19 13:10 UTC, Antoine Jacoutot
Details | Splinter Review
trust: Don't use POSIX or GNU basename() (7.28 KB, patch)
2013-03-19 13:51 UTC, Stef Walter
Details | Splinter Review

Description Antoine Jacoutot 2013-03-18 16:44:45 UTC
Hi.

Using p11-kit-0.16.4 on OpenBSD.

When running seahorse, I'm getting a segfault in strlen (infamous Address 0xfoobar out of bounds).
It seems p11-kit has trouble parsing the system anchors file on OpenBSD (/etc/ssl/cert.pem).



(gdb) run
Starting program: /usr/local/bin/seahorse 
** Message: init gpgme version 1.3.1
[New process 8567]

Program received signal SIGSEGV, Segmentation fault.
[Switching to thread 1002831]
0x000008e9c4aa15f2 in strlen (str=0xffffffffc4ec63c0 <Address 0xffffffffc4ec63c0 out of bounds>) at /usr/src/lib/libc/string/strlen.c:43
43              for (s = str; *s; ++s)
(gdb) bt
#0  0x000008e9c4aa15f2 in strlen (str=0xffffffffc4ec63c0 <Address 0xffffffffc4ec63c0 out of bounds>) at /usr/src/lib/libc/string/strlen.c:43
#1  0x000008e9d6c8d843 in build_object (parser=0x8e9be32cf00, vclass=3628353480, vid=0x0, vlabel=0xffffffffc4ec63c0 <Address 0xffffffffc4ec63c0 out of bounds>) at parser.c:171
#2  0x000008e9d6c8e80c in build_der_extension (parser=0x8e9be32cf00, cert=0x8e9bd7bd600, oid_der=0x8e9d6da17f9 "\006\003U\035\023\006\003U\035%\006\n+\006\001\004\001\231w\006\n\001\006\b+\006\001\005\005\a\003\001", vcritical=0 '\0', 
    ext_der=0x8e9bbd62350 "0\003\001\001� New Roman", ext_len=5) at parser.c:464
#3  0x000008e9d6c8e9dc in build_stapled_extension (parser=0x8e9be32cf00, cert=0x8e9bd7bd600, oid=0x8e9d6da17f9 "\006\003U\035\023\006\003U\035%\006\n+\006\001\004\001\231w\006\n\001\006\b+\006\001\005\005\a\003\001", critical=0 '\0', ext=0x8e9c029ab00) at parser.c:497
#4  0x000008e9d6c8ef9a in build_bc_extension (parser=0x8e9be32cf00, cert=0x8e9bd7bd600, critical=0 '\0', is_ca=1) at parser.c:615
#5  0x000008e9d6c8f187 in update_category (parser=0x8e9be32cf00, cert=0x8e9bd7bd600) at parser.c:681
#6  0x000008e9d6c8f4cc in p11_parsing_update_certificate (parser=0x8e9be32cf00, parsing=0x8e9bd7bb380) at parser.c:768
#7  0x000008e9d6c8d620 in finish_parsing (parser=0x8e9be32cf00, cert_asn=0x8e9c029a300) at parser.c:119
#8  0x000008e9d6c8e6ff in parse_der_x509_certificate (parser=0x8e9be32cf00, data=0x8e9be32f000 "0\202\003U0\202\002=\002\002\001�0\r\006\t*\206H\206�\r\001\001\005\005", length=857) at parser.c:442
#9  0x000008e9d6c8fb00 in on_pem_block (type=0x8e9c37229f0 "CERTIFICATE", contents=0x8e9be32f000 "0\202\003U0\202\002=\002\002\001�0\r\006\t*\206H\206�\r\001\001\005\005", length=857, user_data=0x8e9be32cf00) at parser.c:940
#10 0x000008e9d6c99db5 in p11_pem_parse (
    data=0x8e9c3db01ba "END CERTIFICATE-----\nCertificate:\n    Data:\n        Version: 1 (0x0)\n        Serial Number: 424 (0x1a8)\n    Signature Algorithm: sha1WithRSAEncryption\n        Issuer: C=US, O=GTE Corporation, OU=GTE C"..., n_data=278669, 
    sink=0x8e9d6c8fabb <on_pem_block>, user_data=0x8e9be32cf00) at pem.c:228
#11 0x000008e9d6c8fbb0 in parse_pem_certificates (parser=0x8e9be32cf00, 
    data=0x8e9c3daf000 "Certificate:\n    Data:\n        Version: 3 (0x2)\n        Serial Number: 438 (0x1b6)\n    Signature Algorithm: sha1WithRSAEncryption\n        Issuer: C=US, O=GTE Corporation, OU=GTE CyberTrust Solutions, "..., length=283207) at parser.c:961
#12 0x000008e9d6c8fd97 in p11_parse_memory (parser=0x8e9be32cf00, filename=0x8e9bd7bb660 "/etc/ssl/cert.pem", flags=1, 
    data=0x8e9c3daf000 "Certificate:\n    Data:\n        Version: 3 (0x2)\n        Serial Number: 438 (0x1b6)\n    Signature Algorithm: sha1WithRSAEncryption\n        Issuer: C=US, O=GTE Corporation, OU=GTE CyberTrust Solutions, "..., length=283207, 
    sink=0x8e9d6c93088 <on_parser_object>, sink_data=0x8e9bd796180) at parser.c:1025
#13 0x000008e9d6c8fe94 in p11_parse_file (parser=0x8e9be32cf00, filename=0x8e9bd7bb660 "/etc/ssl/cert.pem", flags=1, sink=0x8e9d6c93088 <on_parser_object>, sink_data=0x8e9bd796180) at parser.c:1056
#14 0x000008e9d6c93179 in loader_load_file (token=0x8e9bd796180, filename=0x8e9bd7bb660 "/etc/ssl/cert.pem", sb=0x8e9b7494960, flags=1) at token.c:94
#15 0x000008e9d6c934c6 in loader_load_path (token=0x8e9bd796180, path=0x8e9bd7bb660 "/etc/ssl/cert.pem", flags=1) at token.c:173
#16 0x000008e9d6c935cf in loader_load_paths (token=0x8e9bd796180, paths=0x0, flags=1) at token.c:202
#17 0x000008e9d6c94432 in p11_token_load (token=0x8e9bd796180) at token.c:395
#18 0x000008e9d6c91d5d in sys_C_FindObjectsInit (handle=16, template=0x8e9bbd60ec0, count=2) at module.c:961
#19 0x000008e9b4f6372f in state_find (args=0x8e9b733bc00, forward=Variable "forward" is not available.
) at gck-enumerator.c:453
#20 0x000008e9b4f62dbf in perform_enumerate_next (args=0x8e9bbd60ac0) at gck-enumerator.c:804
#21 0x000008e9b4f751f1 in process_async_call (data=Variable "data" is not available.
) at gck-call.c:118
#22 0x000008e9c7cf72e6 in g_thread_pool_thread_proxy () from /usr/local/lib/libglib-2.0.so.3400.0
#23 0x000008e9c7cf6525 in g_thread_proxy () from /usr/local/lib/libglib-2.0.so.3400.0
#24 0x000008e9c639511e in _rthread_start (v=Variable "v" is not available.
) at /usr/src/lib/librthread/rthread.c:122
#25 0x000008e9c4a2601b in __tfork_thread () at /usr/src/lib/libc/arch/amd64/sys/tfork_thread.S:75
Cannot access memory at address 0x8e9b7495000
Comment 1 Stef Walter 2013-03-18 18:57:20 UTC
Could you attach the cert.pem for which this is occuring?
Comment 2 Antoine Jacoutot 2013-03-18 19:17:28 UTC
(In reply to comment #1)
> Could you attach the cert.pem for which this is occuring?

Sure. But I guess the easier is to get it directly from:
http://www.openbsd.org/cgi-bin/cvsweb/~checkout~/src/lib/libssl/cert.pem?rev=1.22;content-type=text%2Fplain
Comment 3 Stef Walter 2013-03-19 11:48:18 UTC
That's so strange. I can't duplicate this. Could you help me with a bit of debugging? In the stack trace below could you go up to build_object() (ie: #1) and print out the value of parser->basename

You can also reach me at #p11-kit on freenode so we can do a bit of back and forth on this.

It's also worth noting that in 0.17.x much of this code has been rewritten. But I'd still like to figure out why this code is crashing on OpenBSD so we can prevent a similar thing from happening.
Comment 4 Antoine Jacoutot 2013-03-19 12:03:48 UTC
(In reply to comment #3)
> That's so strange. I can't duplicate this. Could you help me with a bit of
> debugging? In the stack trace below could you go up to build_object() (ie:
> #1) and print out the value of parser->basename

Sure thing.
(gdb) print parser->basename
$1 = 0xffffffffb33f23c0 <Address 0xffffffffb33f23c0 out of bounds>

> You can also reach me at #p11-kit on freenode so we can do a bit of back and
> forth on this.

Nice. I'll join in a few; just ping me when you have some time, I'll be idling :-)
Comment 5 Antoine Jacoutot 2013-03-19 13:08:42 UTC
Ok I found the issue.
trust/parser.c is missing an include for libgen.h which basename(3) requires.
(implicit declaration of function 'basename')

The included patch fixes crash for me.
Comment 6 Antoine Jacoutot 2013-03-19 13:10:33 UTC
Created attachment 76753 [details] [review]
add missing include for libgen.h
Comment 7 Stef Walter 2013-03-19 13:51:34 UTC
Created attachment 76755 [details] [review]
trust: Don't use POSIX or GNU basename()
Comment 8 Stef Walter 2013-03-19 13:52:23 UTC
Comment on attachment 76753 [details] [review]
add missing include for libgen.h

Review of attachment 76753 [details] [review]:
-----------------------------------------------------------------

Besides this nastiness with the headers changing how basename() works, it's also not thread safe. So let's do our own. I've attached such a patch.
Comment 9 Antoine Jacoutot 2013-03-19 14:23:10 UTC
> Besides this nastiness with the headers changing how basename() works, it's
> also not thread safe. So let's do our own. I've attached such a patch.

Works beautifully, thank you.
Comment 10 Stef Walter 2013-03-19 15:47:33 UTC
Attachment 76755 [details] pushed as 7c27e9f - trust: Don't use POSIX or GNU basename()


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.