brianmario / yajl-ruby

A streaming JSON parsing and encoding library for Ruby (C bindings to yajl)
http://rdoc.info/projects/brianmario/yajl-ruby
MIT License
1.48k stars 169 forks source link

SIGABRT - process aborted #176

Closed clod81 closed 7 years ago

clod81 commented 7 years ago

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 commented 7 years ago

What version of yajl-ruby are you using?

clod81 commented 7 years ago

latest available on rubygems, 1.3.0

carnil commented 7 years ago

This has been assigned CVE-2017-16516

brianmario commented 7 years ago

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 commented 7 years ago

I put up the test program I was using here.

brianmario commented 7 years ago

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 commented 7 years ago

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

carnil commented 7 years ago

a8ca8f476655adaa187eedc60bdc770fff3c51ce seems to address the issue

clod81 commented 7 years ago

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

brianmario commented 7 years ago

Of course!

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

clod81 commented 7 years ago

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

brianmario commented 7 years ago

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

Gaurav28102 commented 7 months ago

Good job