Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SIGABRT - process aborted #176

Closed
clod81 opened this issue Nov 2, 2017 · 13 comments · Fixed by #178
Closed

SIGABRT - process aborted #176

clod81 opened this issue Nov 2, 2017 · 13 comments · Fixed by #178

Comments

@clod81
Copy link

clod81 commented Nov 2, 2017

PoC:

require 'yajl'

File.open(ARGV[0]) do |f|
  Yajl::Parser.new.parse(f)
end

File passed as input:
{"e":{"\uD800\\DC00":"a"}}

Output:

ruby: yajl_encode.c:192: void yajl_string_decode(yajl_buf, const unsigned char *, unsigned int): Assertion `"this should never happen" == NULL' failed.

With this input:

{"\uDBFF\\u "@: "

Output:

ruby: yajl_encode.c:108: void hexToDigit(unsigned int *, const unsigned char *): Assertion `!(c & 0xF0)' failed.
@brianmario
Copy link
Owner

What version of yajl-ruby are you using?

@clod81
Copy link
Author

clod81 commented Nov 2, 2017

latest available on rubygems, 1.3.0

@carnil
Copy link

carnil commented Nov 3, 2017

This has been assigned CVE-2017-16516

@brianmario
Copy link
Owner

yajl-ruby embeds a patched copy of yajl itself. I was able to reproduce this on my machine issue on the yajl 1.x branch, but curiously only if I enabled debug symbols 🤔

I'm running macOS 10.13.1 and the latest Xcode developer tools.

You should be able to reproduce it by cloning yajl locally, checking out the 1.x branch then building the library like so:

$ cmake -DCMAKE_BUILD_TYPE=Debug
$ make

From there you'll need to build a small C program that links against the library and reads the bad input you specified in this issue.

I'll continue to debug this to try and figure out a fix, but in the meantime I think we should get an issue opened over on the yajl repo itself. What do you think?

@brianmario
Copy link
Owner

I put up the test program I was using here.

@brianmario
Copy link
Owner

I was actually able to reproduce this on yajl master as well 😞

Still digging but it seems like it might be that there's no validation that the character following a surrogate pair is in the expected escaped format.

@brianmario
Copy link
Owner

The smallest reproduction case is "\uD800\\1" where "1" there can literally be any character.

brianmario added a commit that referenced this issue Nov 7, 2017
If a valid surrogate character escape is found, but the following
byte sequence isn't a valid unicode escape sequence, insert our
replacement character '?' as we would any other place we saw
invalid characters while unescaping.

Fixes #176
@carnil
Copy link

carnil commented Nov 8, 2017

a8ca8f4 seems to address the issue

@clod81
Copy link
Author

clod81 commented Nov 8, 2017

I ran the new release against the two test cases, and it's looking good. Thanks for the quick turnaround @brianmario 👍

@brianmario
Copy link
Owner

Of course!

Though next time (but hopefully there is no "next time" haha) let's make sure to disclose vulnerabilities privately first 😉

@clod81
Copy link
Author

clod81 commented Nov 8, 2017

@brianmario didn't notice your details were on your profile, but ok.

@brianmario
Copy link
Owner

Oh ok no worries, glad we were able to get it fixed so quickly 😄

netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this issue Dec 9, 2017
No upstream changelog, but seems to include security fixes CVE-2017-16516
and others:
 brianmario/yajl-ruby#176
 brianmario/yajl-ruby#178
@Gaurav28102
Copy link

Good job

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants