search

Southern / node-x509

Simple X509 certificate parser.
105 stars 84 forks source link

SegFault on certain certificates #37

Open mwpnl opened 8 years ago

mwpnl commented 8 years ago

When parsing certain certificates, for some reason a segmentation fault occurs that comes from the x509 module. When parsing the certificate directly from the CLI with openSSL no error occurs.

An example of the certificate is the one added as cert.txt: (rename to cert.pem) cert.txt

When executing on:

the following stack trace occurs:

PID 10938 received SIGSEGV for address: 0x0 /root/test/node_modules/segfault-handler/build/Release/segfault-handler.node(+0x1a7b)[0x7f6556050a7b] /lib64/libpthread.so.0(+0xf100)[0x7f655882f100] /lib64/libc.so.6(+0x13aa4f)[0x7f6558598a4f] node(BUF_strlcpy+0x50)[0x8181c0] /root/test/node_modules/x509/build/Release/x509.node(_Z9try_parseRKSs+0x965)[0x7f6556257115] /root/test/node_modules/x509/build/Release/x509.node(_Z10parse_certRKN3Nan20FunctionCallbackInfoIN2v85ValueEEE+0x58)[0x7f6556257b88] /root/test/node_modules/x509/build/Release/x509.node(+0x2b96)[0x7f6556255b96] node(_ZN2v88internal25FunctionCallbackArguments4CallEPFvRKNS_20FunctionCallbackInfoINS_5ValueEEEE+0xfe)[0x9484ae] node[0x993bfd] node[0x99414a] [0x288f8890961b]

The simple node.js application used to execute the X509 is the following (I've added the node-segfault-handler module to make sure I get the above stack trace):

test.txt (rename to test.js).

If I execute openssl x509 -in cert.pem from the CLI I get the expected parsed result of the certificate.

The error occurs with only a few certificates, almost all other certificates are parsed with ease, so any idea on what could be wrong or how to fix this would be much appreciated.

Rush commented 8 years ago

Sorry to say but even demangled, this stacktrace seems quite useless:

PID 10938 received SIGSEGV for address: 0x0 /root/test/node_modules/segfault-handler/build/Release/segfault-handler.node( 0x1a7b)[0x7f6556050a7b] /lib64/libpthread.so.0( 0xf100)[0x7f655882f100]

 /lib64/libc.so.6( 0x13aa4f)[0x7f6558598a4f] node(BUF_strlcpy 0x50)[0x8181c0] /root/test/node_modules/x509/build/Release/x509.node(try_parse(std::string const&) 0x965)[0x7f6556257115] 

/root/test/node_modules/x509/build/Release/x509.node(parse_cert(Nan::FunctionCallbackInfo<v8::Value> const&) 0x58)[0x7f6556257b88] /root/test/node_modules/x509/build/Release/x509.node( 0x2b96)[0x7f6556255b96]

node(v8::internal::FunctionCallbackArguments::Call(void (*)(v8::FunctionCallbackInfo<v8::Value> const&)) 0xfe)[0x9484ae] node[0x993bfd] node[0x99414a] [0x288f8890961b]

It would be helpful if you could compile node-x509 in debug mode. git clone it from github to somewhere and inside do npm install && node-gyp rebuild --debug. If you're missing the node-gyp tool then you can install it with npm install -g node-gyp (if you use NVM you can do it as a normal user).

In your application directory do rm -rf node_modules/x509 && ln -rs /path/to/x509/repo node_modules/x509.

Now you can produce some useful outputs:

$ valgrind node test-program.js cert.crt

and

$ gdb --args node test-program.js cert.crt
> run
# after it crashes, write the below
> bt

Paste the resulting stacktraces here and I'm sure @Southern will be able to assist.

mwpnl commented 8 years ago

Thanks! That's very fast and a helpful guide on how to (re)build a module in debug mode. I'll post the results when I have them :)

mwpnl commented 8 years ago

I'm not quite sure if adding the complete stack in-line was what you had in mind, but if it's easier to put the stack in a separate file I figured I can always edit this comment and do it afterwards.

Does this provide enough information be be useful? I'd love to be able to help out more, but my knowledge on this matter is too limited.

These are the results from running valgrind:

==12698== Memcheck, a memory error detector
==12698== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==12698== Using Valgrind-3.10.0 and LibVEX; rerun with -h for copyright info
==12698== Command: node test.js
==12698==
==12698== Warning: set address range perms: large range [0x3d97f207a000, 0x3d981207a000) (noaccess)
==12698== Conditional jump or move depends on uninitialised value(s)
==12698==    at 0x4C2CBA9: strlen (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==12698==    by 0x8181BF: BUF_strlcpy (in /usr/bin/node)
==12698==    by 0x984E14B: parse_date(asn1_string_st*) (x509.cc:335)
==12698==    by 0x984D32E: try_parse(std::string const&) (x509.cc:156)
==12698==    by 0x984CE8C: parse_cert(Nan::FunctionCallbackInfo const&) (x509.cc:90)
==12698==    by 0x984A834: Nan::imp::FunctionCallbackWrapper(v8::FunctionCallbackInfo const&) (nan_callbacks_12_inl.h:174)
==12698==    by 0x9484AD: v8::internal::FunctionCallbackArguments::Call(void (*)(v8::FunctionCallbackInfo const&)) (in /usr/bin/node)
==12698==    by 0x993BFC: ??? (in /usr/bin/node)
==12698==    by 0x994149: ??? (in /usr/bin/node)
==12698==    by 0x3D97F240961A: ???
==12698==    by 0x3D97F254D2BE: ???
==12698==    by 0x3D97F253E160: ???
==12698==
==12698== Conditional jump or move depends on uninitialised value(s)
==12698==    at 0x4C2CBB8: strlen (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==12698==    by 0x8181BF: BUF_strlcpy (in /usr/bin/node)
==12698==    by 0x984E14B: parse_date(asn1_string_st*) (x509.cc:335)
==12698==    by 0x984D32E: try_parse(std::string const&) (x509.cc:156)
==12698==    by 0x984CE8C: parse_cert(Nan::FunctionCallbackInfo const&) (x509.cc:90)
==12698==    by 0x984A834: Nan::imp::FunctionCallbackWrapper(v8::FunctionCallbackInfo const&) (nan_callbacks_12_inl.h:174)
==12698==    by 0x9484AD: v8::internal::FunctionCallbackArguments::Call(void (*)(v8::FunctionCallbackInfo const&)) (in /usr/bin/node)
==12698==    by 0x993BFC: ??? (in /usr/bin/node)
==12698==    by 0x994149: ??? (in /usr/bin/node)
==12698==    by 0x3D97F240961A: ???
==12698==    by 0x3D97F254D2BE: ???
==12698==    by 0x3D97F253E160: ???
==12698==
==12698== Conditional jump or move depends on uninitialised value(s)
==12698==    at 0x4C2CBA9: strlen (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==12698==    by 0x8181BF: BUF_strlcpy (in /usr/bin/node)
==12698==    by 0x984E14B: parse_date(asn1_string_st*) (x509.cc:335)
==12698==    by 0x984D39B: try_parse(std::string const&) (x509.cc:159)
==12698==    by 0x984CE8C: parse_cert(Nan::FunctionCallbackInfo const&) (x509.cc:90)
==12698==    by 0x984A834: Nan::imp::FunctionCallbackWrapper(v8::FunctionCallbackInfo const&) (nan_callbacks_12_inl.h:174)
==12698==    by 0x9484AD: v8::internal::FunctionCallbackArguments::Call(void (*)(v8::FunctionCallbackInfo const&)) (in /usr/bin/node)
==12698==    by 0x993BFC: ??? (in /usr/bin/node)
==12698==    by 0x994149: ??? (in /usr/bin/node)
==12698==    by 0x3D97F240961A: ???
==12698==    by 0x3D97F254D2BE: ???
==12698==    by 0x3D97F253E160: ???
==12698==
==12698== Conditional jump or move depends on uninitialised value(s)
==12698==    at 0x4C2CBB8: strlen (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==12698==    by 0x8181BF: BUF_strlcpy (in /usr/bin/node)
==12698==    by 0x984E14B: parse_date(asn1_string_st*) (x509.cc:335)
==12698==    by 0x984D39B: try_parse(std::string const&) (x509.cc:159)
==12698==    by 0x984CE8C: parse_cert(Nan::FunctionCallbackInfo const&) (x509.cc:90)
==12698==    by 0x984A834: Nan::imp::FunctionCallbackWrapper(v8::FunctionCallbackInfo const&) (nan_callbacks_12_inl.h:174)
==12698==    by 0x9484AD: v8::internal::FunctionCallbackArguments::Call(void (*)(v8::FunctionCallbackInfo const&)) (in /usr/bin/node)
==12698==    by 0x993BFC: ??? (in /usr/bin/node)
==12698==    by 0x994149: ??? (in /usr/bin/node)
==12698==    by 0x3D97F240961A: ???
==12698==    by 0x3D97F254D2BE: ???
==12698==    by 0x3D97F253E160: ???
==12698==
==12698== Conditional jump or move depends on uninitialised value(s)
==12698==    at 0x4C2CBA9: strlen (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==12698==    by 0x8181BF: BUF_strlcpy (in /usr/bin/node)
==12698==    by 0x984DD17: try_parse(std::string const&) (x509.cc:282)
==12698==    by 0x984CE8C: parse_cert(Nan::FunctionCallbackInfo const&) (x509.cc:90)
==12698==    by 0x984A834: Nan::imp::FunctionCallbackWrapper(v8::FunctionCallbackInfo const&) (nan_callbacks_12_inl.h:174)
==12698==    by 0x9484AD: v8::internal::FunctionCallbackArguments::Call(void (*)(v8::FunctionCallbackInfo const&)) (in /usr/bin/node)
==12698==    by 0x993BFC: ??? (in /usr/bin/node)
==12698==    by 0x994149: ??? (in /usr/bin/node)
==12698==    by 0x3D97F240961A: ???
==12698==    by 0x3D97F254D2BE: ???
==12698==    by 0x3D97F253E160: ???
==12698==    by 0x3D97F253DDAF: ???
==12698==
==12698== Invalid read of size 1
==12698==    at 0x4C2CBA2: strlen (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==12698==    by 0x8181BF: BUF_strlcpy (in /usr/bin/node)
==12698==    by 0x984DD17: try_parse(std::string const&) (x509.cc:282)
==12698==    by 0x984CE8C: parse_cert(Nan::FunctionCallbackInfo const&) (x509.cc:90)
==12698==    by 0x984A834: Nan::imp::FunctionCallbackWrapper(v8::FunctionCallbackInfo const&) (nan_callbacks_12_inl.h:174)
==12698==    by 0x9484AD: v8::internal::FunctionCallbackArguments::Call(void (*)(v8::FunctionCallbackInfo const&)) (in /usr/bin/node)
==12698==    by 0x993BFC: ??? (in /usr/bin/node)
==12698==    by 0x994149: ??? (in /usr/bin/node)
==12698==    by 0x3D97F240961A: ???
==12698==    by 0x3D97F254D2BE: ???
==12698==    by 0x3D97F253E160: ???
==12698==    by 0x3D97F253DDAF: ???
==12698==  Address 0x0 is not stack'd, malloc'd or (recently) free'd
==12698==
PID 12698 received SIGSEGV for address: 0x0
/root/test/node_modules/segfault-handler/build/Release/segfault-handler.node(+0x1a7b)[0x9a54a7b]
/lib64/libpthread.so.0(+0xf100)[0x5a70100]
/usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so(_vgr20070ZU_libcZdsoZa_strlen+0x2)[0x4c2cba2]
node(BUF_strlcpy+0x50)[0x8181c0]
/root/node-x509/build/Debug/x509.node(_Z9try_parseRKSs+0xe32)[0x984dd18]
/root/node-x509/build/Debug/x509.node(_Z10parse_certRKN3Nan20FunctionCallbackInfoIN2v85ValueEEE+0x69)[0x984ce8d]
/root/node-x509/build/Debug/x509.node(+0x7835)[0x984a835]
node(_ZN2v88internal25FunctionCallbackArguments4CallEPFvRKNS_20FunctionCallbackInfoINS_5ValueEEEE+0xfe)[0x9484ae]
node[0x993bfd]
node[0x99414a]
[0x3d97f240961b]
==12698==
==12698== HEAP SUMMARY:
==12698==     in use at exit: 439,217 bytes in 575 blocks
==12698==   total heap usage: 15,852 allocs, 15,277 frees, 44,073,888 bytes allocated
==12698==
==12698== LEAK SUMMARY:
==12698==    definitely lost: 121 bytes in 2 blocks
==12698==    indirectly lost: 52 bytes in 2 blocks
==12698==      possibly lost: 22,668 bytes in 131 blocks
==12698==    still reachable: 416,376 bytes in 440 blocks
==12698==         suppressed: 0 bytes in 0 blocks
==12698== Rerun with --leak-check=full to see details of leaked memory
==12698==
==12698== For counts of detected and suppressed errors, rerun with: -v
==12698== Use --track-origins=yes to see where uninitialised values come from
==12698== ERROR SUMMARY: 12 errors from 6 contexts (suppressed: 1 from 1)

And when running gdb, the following trace occurs (not sure if anything after number 7 is relevant):

(gdb) run
Starting program: /usr/bin/node test.js
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".
[New Thread 0x7ffff7ff8700 (LWP 12719)]
[New Thread 0x7ffff6bd0700 (LWP 12720)]
[New Thread 0x7ffff63cf700 (LWP 12721)]
[New Thread 0x7ffff5bce700 (LWP 12722)]
[New Thread 0x7ffff53cd700 (LWP 12723)]
Program received signal SIGSEGV, Segmentation fault.
0x00007ffff6d0ba4f in __strlen_sse42 () from /lib64/libc.so.6
(gdb) bt
#0  0x00007ffff6d0ba4f in __strlen_sse42 () from /lib64/libc.so.6
#1  0x00000000008181c0 in BUF_strlcpy ()
#2  0x00007ffff49c7d18 in try_parse (dataString="/root/test/cert.pem") at ../src/x509.cc:282
#3  0x00007ffff49c6e8d in parse_cert (info=...) at ../src/x509.cc:90
#4  0x00007ffff49c4835 in Nan::imp::FunctionCallbackWrapper (info=...) at ../node_modules/nan/nan_callbacks_12_inl.h:174
#5  0x00000000009484ae in v8::internal::FunctionCallbackArguments::Call(void (*)(v8::FunctionCallbackInfo const&)) ()
#6  0x0000000000993bfd in v8::internal::MaybeHandle v8::internal::(anonymous namespace)::HandleApiCallHelper(v8::internal::Isolate*, v8::internal::(anonymous namespace)::BuiltinArguments<(v8::internal::BuiltinExtraArguments)1>) ()
#7  0x000000000099414a in v8::internal::Builtin_HandleApiCall(int, v8::internal::Object**, v8::internal::Isolate*) ()
#8  0x00002776b2c0961b in ?? ()
#9  0x00002776b2c09561 in ?? ()
#10 0x00007fffffffda90 in ?? ()
#11 0x00007fffffffdb08 in ?? ()
#12 0x00002776b2d4d2bf in ?? ()
#13 0x000033677bf61131 in ?? ()
#14 0x000033677bf5b239 in ?? ()
#15 0x0000243e8e0dba89 in ?? ()
#16 0x000033677bf61131 in ?? ()
#17 0x0000361a89204189 in ?? ()
#18 0x0000361a89204189 in ?? ()
#19 0x0000361a89204189 in ?? ()
#20 0x0000361a89204189 in ?? ()
#21 0x0000361a89204189 in ?? ()
#22 0x000033677bf611c9 in ?? ()
#23 0x0000243e8e0dab11 in ?? ()
#24 0x00007fffffffdb58 in ?? ()
#25 0x00002776b2d3e161 in ?? ()
#26 0x000033677bf5b239 in ?? ()
#27 0x0000243e8e0d8bf9 in ?? ()
#28 0x000033677bf611c9 in ?? ()
#29 0x0000361a89204189 in ?? ()
#30 0x0000243e8e0f6769 in ?? ()
#31 0x0000243e8e0d8bf9 in ?? ()
#32 0x0000243e8e0d6489 in ?? ()
#33 0x0000361a892ac501 in ?? ()
#34 0x00007fffffffdc08 in ?? ()
#35 0x00002776b2d3ddb0 in ?? ()
#36 0x0000243e8e0d64e9 in ?? ()
#37 0x0000243e8e0d4749 in ?? ()
#38 0x0000243e8e0d47d1 in ?? ()
#39 0x0000243e8e0d65c1 in ?? ()
#40 0x0000243e8e0d48c9 in ?? ()
#41 0x0000243e8e0d48c9 in ?? ()
#42 0x0000361a892ba011 in ?? ()
#43 0x0000361a89204189 in ?? ()
#44 0x0000000000000000 in ?? ()
Rush commented 8 years ago

Almost good but there are still no line numbers as I see that a Release version is being loaded /root/node-x509/build/Release/x509.node. We need there to be Debug. I suspect that node-gyp rebuild --debug did not work for some reason. Also you can try removing the Release version by hand.

mwpnl commented 8 years ago

It took me a while to find out what you ment with the line numbers, but I think I've got it right this time!

I updated my comment above with the fresh stack traces with line numbers to keep the discussion clean, hope that's okay.

Southern commented 8 years ago

I'm pretty busy for the next couple of days. It'll probably be the weekend before I can get around to it.

@yorkie Do you have any free time to check this out?

yorkie commented 8 years ago

@mwpnl @Southern after taking a look at gdb results, I think the potential missing line is here:

char *data = (char*) malloc(bptr->length + 1);
BUF_strlcpy(data, bptr->data, bptr->length + 1);

We didn't check if the data has been able to access after calling malloc, therefore, @mwpnl may I ask you to update the "src/x509.cc" at line 281 as the following diff:

diff --git a/src/x509.cc b/src/x509.cc
index acad881..016f45d 100644
--- a/src/x509.cc
+++ b/src/x509.cc
@@ -279,6 +279,12 @@ Local<Value> try_parse(const std::string& dataString) {
     BIO_set_close(ext_bio, BIO_CLOSE);

     char *data = (char*) malloc(bptr->length + 1);
+    if (data == NULL) {
+      BIO_free(ext_bio);
+      printf("malloc in try_parse result is not accessible\n");
+      break;
+    }
+
     BUF_strlcpy(data, bptr->data, bptr->length + 1);
     data = trim(data, bptr->length);

And try to run with gdb again, then we will see what happened in details, thanks :-)

mwpnl commented 8 years ago

@yorkie was completely right, the issue was with a bptr->data that was completely empty. I've adapted the above code diff and made pull request #39, that solves this issue in the certs I mentioned above.

Looking forward to see your comments on this!

yorkie commented 8 years ago

Okay @mwpnl will comment when I get home later, thanks :)

jthomerson commented 6 years ago

@Southern what's the status of this issue? I see on the PR #39 that you said

With the lack of activity in this PR, I'm starting to believe this was a case of a certificate being malformed or some other factor than this specific module and was solved by correcting that issue.

Maybe it is from a malformed certificate, but still we need the x509 module to avoid a segfault when reading such a certificate. I'm trying to read through all certificate transparency logs and I'm getting a segfault when reading a different certificate. Ironically (or not), the cert is by the same issuer and for the same subject as the one @mwpnl originally reported with this issue, although it's a different cert.

I've pasted the cert below. You can also see details about it at https://crt.sh/?id=6039630. I've actually tried every cert that's in the CT logs that has that subject common name (https://crt.sh/?q=Cybertrust+Japan+EV+CA+G2+OCSP+Responder), and every one of them fails exactly the same.

-----BEGIN CERTIFICATE-----
MIIDzjCCAragAwIBAgIUN5EK0B6cCK/IL6jSmZ6ifu6olsMwDQYJKoZIhvcNAQEF
BQAwVjELMAkGA1UEBhMCSlAxIzAhBgNVBAoTGkN5YmVydHJ1c3QgSmFwYW4gQ28u
LCBMdGQuMSIwIAYDVQQDExlDeWJlcnRydXN0IEphcGFuIEVWIENBIEcyMB4XDTE0
MTEyODA4MzgyNFoXDTE2MTEzMDE0NTk1OVowZTELMAkGA1UEBhMCSlAxIzAhBgNV
BAoTGkN5YmVydHJ1c3QgSmFwYW4gQ28uLCBMdGQuMTEwLwYDVQQDEyhDeWJlcnRy
dXN0IEphcGFuIEVWIENBIEcyIE9DU1AgUmVzcG9uZGVyMIIBIjANBgkqhkiG9w0B
AQEFAAOCAQ8AMIIBCgKCAQEA2P4xPrFd60ZbUfGe3YQG9DGgkSC1DKFIOosVct0x
5uasy+EIfdXy2L1Jicd4RlLMdkCfm/6f4DOKOQu174mxLHL+NcHlGU8+iFtBkmvp
sA8sonqGgfkaEbyJLuQbWAkbCG/uZfpdQz87aAN2ouqRvxweW+bJON50P3N95LsA
C9dZR2+GTeP/WY5hC/4DkUYlv+bl96xW6cHr31ViF/2QwGjwQuLIa9IN4+Ll5BV3
8pOFVbKpy9qecISAdXj4COoOpumz871sItHK7olNJQvpClu558SUsyZk3ydDI8gH
yjzu8I97UmxcMYs3/ughyWx1zz385lXNLLPTlxV3JXFe+QIDAQABo4GEMIGBMAkG
A1UdEwQCMAAwDwYJKwYBBQUHMAEFBAIFADAOBgNVHQ8BAf8EBAMCB4AwEwYDVR0l
BAwwCgYIKwYBBQUHAwkwHwYDVR0jBBgwFoAUkUMF7LRqFU/c4e6GVlwR0CorjV8w
HQYDVR0OBBYEFMXTVhjIBJtS0b9ry+4DcEutk/inMA0GCSqGSIb3DQEBBQUAA4IB
AQBa+iAO9iie7cNno4RF2Tx+gt85SZKxsRA9ZOiU8dbAx8+gCE3Jfjp3UKZoW04Z
698WAh3CDkofJmg5E2g/hgt+YQ/2rZU70pFYO8T0NOemOOUbSgzw2rKt3Vk3SjsU
ciyo8osNG1ZaQEn1wXkur3pYuaKFeh28bSS6QjiI7I+KK02VGOtI+DclGzk0OU5O
ePl2rLTC0Y7o+JVMk6bjyhGhY1O2JWriYzeLLvPSZh0T13/Yes55S8eda0maFPYg
xspv/+0a1AMB+3aBM0HZQmKZf9mAHKDCDZtqrOfBKVUJ2MIeKnxPW9730HFAlMiR
XakqqIPar5fIWlZum0Jb0lQy
-----END CERTIFICATE-----

Test script:

var x509 = require('x509'),
    cert = require('fs').readFileSync(process.argv[2]).toString();

console.log(cert);
console.log(x509.parseCert(cert));

Run it with node test.js ~/Downloads/6039630.crt after downloading one of the certs from crt.sh for example.

Thanks!

jthomerson commented 6 years ago

It should be noted that every one of those certs parses fine with openssl on the CLI, so optimally x509 would parse them as well, returning the same info that openssl does.

mwpnl commented 6 years ago

@jthomerson Looping through the CT log is exactly how I came about this issue. Great find that they're all certificates with the came CN!

After my initial PR #39 another user made a similar request in PR #56.

The - to me - cryptic review comments by @yorkie on my PR made me abandon the PR. With the best intentions of the word, I fail to see what's wrong. And, the solution works for me, travelling through the CT log.

If anyone would be willing to elaborate some more on the proposed fixes, I'd me more than happy to make those changes anew.