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

json::parse() ubsan regression with v3.7.0 #1716

Closed
bassosimone opened this issue Aug 21, 2019 · 3 comments · Fixed by #1728
Closed

json::parse() ubsan regression with v3.7.0 #1716

bassosimone opened this issue Aug 21, 2019 · 3 comments · Fixed by #1728
Assignees
Labels
kind: bug release item: 🔨 further change solution: proposed fix a fix for the issue has been proposed and waits for confirmation
Milestone

Comments

@bassosimone
Copy link

bassosimone commented Aug 21, 2019

  • What is the issue you have?

When upgrading from v3.6.1 to v3.7.0, I did see a crash in the CI of a library I maintain when using the undefined behaviour sanitiser with GCC. I was able to cut down complexity and produce a minimal test case that shows the issue is around json::parse() (see below).

  • Please describe the steps to reproduce the issue. Can you provide a small but working code example?
  1. docker run -it debian:testing-20190812; then inside the container do:
  2. apt update
  3. apt install -y g++ nvi wget
  4. wget https://github.com/nlohmann/json/releases/download/v3.7.0/json.hpp
  5. edit main.cpp to contain the code shown below
  6. g++ -fsanitize=undefined -fno-sanitize-recover main.cpp
  7. ./a.out

where:

// main.cpp
#include "json.hpp"
int main() {
  std::string m = "";
  auto doc = nlohmann::json::parse(m);
}
  • What is the expected behavior?

This is what I obtain with nlohmann/[email protected] as well as with v3.7.1 when (1) not using the sanitiser or (2) m is a const char *:

# ./a.out
terminate called after throwing an instance of 'nlohmann::detail::parse_error'
  what():  [json.exception.parse_error.101] parse error at line 1, column 1: syntax
    error while parsing value - unexpected end of input; expected '[', '{', or a literal
Aborted
  • And what is the actual behavior instead?
# ./a.out
/usr/include/c++/8/ext/new_allocator.h:136:4: runtime error: null pointer passed as
    argument 2, which is declared to never be null

See the bottom of this issue for a stacktrace.

Compiler:

# g++ --version
g++ (Debian 8.3.0-19) 8.3.0
[snip]

System: debian:testing-20190812. Upon arriving in this point of the bug report, I also attempted to reproduce the issue with the debian:stable image, with g++ (Debian 8.3.0-6) 8.3.0, where the behaviour was exactly the same.

  • Did you use a released version of the library or the version from the develop branch?

I did test v3.6.1, v3.7.0, and a015b78. Only v0.3.6.1 was working as intended.

I don't think this question applies to my case.


From a debian:stable docker container, and with v3.7.0, I obtained this stack trace (which has been edited for wrapping long lines):

# head -n 10 json.hpp 
/*
    __ _____ _____ _____
 __|  |   __|     |   | |  JSON for Modern C++
|  |  |__   |  |  | | | |  version 3.7.0
|_____|_____|_____|_|___|  https://github.com/nlohmann/json

Licensed under the MIT License <http://opensource.org/licenses/MIT>.
SPDX-License-Identifier: MIT
Copyright (c) 2013-2019 Niels Lohmann <http://nlohmann.me>.

# cat main.cpp
// main.cpp
#include "json.hpp"
int main() {
  std::string m = "";
  auto doc = nlohmann::json::parse(m);
}

# g++ -g -fno-omit-frame-pointer -fsanitize=undefined -fno-sanitize-recover main.cpp

# export UBSAN_OPTIONS=print_stacktrace=1

# ./a.out 
/usr/include/c++/8/ext/new_allocator.h:136:4: runtime error: null pointer
  passed as argument 2, which is declared to never be null

    #0 0x55820c2f8650 in void __gnu_cxx::new_allocator<
      nlohmann::detail::input_buffer_adapter>::construct<
        nlohmann::detail::input_buffer_adapter, decltype(nullptr),
          unsigned long const&>(nlohmann::detail::input_buffer_adapter*,
            decltype(nullptr)&&, unsigned long const&)
              /usr/include/c++/8/ext/new_allocator.h:136

    #1 0x55820c2f5c04 in void std::allocator_traits<
      std::allocator<nlohmann::detail::input_buffer_adapter> >::
        construct<nlohmann::detail::input_buffer_adapter, decltype(nullptr),
          unsigned long const&>(std::allocator<nlohmann::detail::input_buffer_adapter>&,
            nlohmann::detail::input_buffer_adapter*, decltype(nullptr)&&,
              unsigned long const&) /usr/include/c++/8/bits/alloc_traits.h:475

    #2 0x55820c2f0986 in std::_Sp_counted_ptr_inplace<
      nlohmann::detail::input_buffer_adapter, std::allocator<
        nlohmann::detail::input_buffer_adapter>, (__gnu_cxx::_Lock_policy)2>::
          _Sp_counted_ptr_inplace<decltype(nullptr),
            unsigned long const&>(std::allocator<nlohmann::detail::input_buffer_adapter>,
              decltype(nullptr)&&, unsigned long const&)
                /usr/include/c++/8/bits/shared_ptr_base.h:545

    #3 0x55820c2e71b9 in std::__shared_count<(__gnu_cxx::_Lock_policy)2>::
      __shared_count<nlohmann::detail::input_buffer_adapter,
        std::allocator<nlohmann::detail::input_buffer_adapter>, decltype(nullptr),
          unsigned long const&>(nlohmann::detail::input_buffer_adapter*&,
            std::_Sp_alloc_shared_tag<std::allocator<
              nlohmann::detail::input_buffer_adapter> >, decltype(nullptr)&&,
                unsigned long const&) /usr/include/c++/8/bits/shared_ptr_base.h:677

    #4 0x55820c2de847 in std::__shared_ptr<
      nlohmann::detail::input_buffer_adapter, (__gnu_cxx::_Lock_policy)2>::
        __shared_ptr<std::allocator<nlohmann::detail::input_buffer_adapter>,
          decltype(nullptr), unsigned long const&>(
            std::_Sp_alloc_shared_tag<std::allocator<nlohmann::detail::
              input_buffer_adapter> >, decltype(nullptr)&&,
                unsigned long const&) /usr/include/c++/8/bits/shared_ptr_base.h:1342

    #5 0x55820c2c9296 in std::shared_ptr<nlohmann::detail::input_buffer_adapter
      >::shared_ptr<std::allocator<nlohmann::detail::input_buffer_adapter>,
        decltype(nullptr), unsigned long const&>(
          std::_Sp_alloc_shared_tag<std::allocator<
            nlohmann::detail::input_buffer_adapter> >, decltype(nullptr)&&,
              unsigned long const&) /usr/include/c++/8/bits/shared_ptr.h:359

    #6 0x55820c2c3461 in std::shared_ptr<nlohmann::detail::input_buffer_adapter>
      std::allocate_shared<nlohmann::detail::input_buffer_adapter,
        std::allocator<nlohmann::detail::input_buffer_adapter>, decltype(nullptr),
          unsigned long const&>(std::allocator<nlohmann::detail::input_buffer_adapter
            > const&, decltype(nullptr)&&, unsigned long const&) 
              /usr/include/c++/8/bits/shared_ptr.h:706

    #7 0x55820c2b637f in std::shared_ptr<nlohmann::detail::input_buffer_adapter
       > std::make_shared<nlohmann::detail::input_buffer_adapter,
         decltype(nullptr), unsigned long const&>(decltype(nullptr)&&,
           unsigned long const&) /usr/include/c++/8/bits/shared_ptr.h:722

    #8 0x55820c2b579f in nlohmann::detail::input_adapter::input_adapter<
      __gnu_cxx::__normal_iterator<char const*, std::__cxx11::basic_string<
       char, std::char_traits<char>, std::allocator<char> > >, 0>
         (__gnu_cxx::__normal_iterator<char const*, std::__cxx11::basic_string<
           char, std::char_traits<char>, std::allocator<char> > >,
             __gnu_cxx::__normal_iterator<char const*, std::__cxx11::basic_string<
               char, std::char_traits<char>, std::allocator<char> > >) //json.hpp:4166

    #9 0x55820c2b22b4 in nlohmann::detail::input_adapter::input_adapter<
      std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, 0>
        (std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >
          const&) //json.hpp:4181

    #10 0x55820c2ab686 in main //main.cpp:5
@t-b
Copy link
Contributor

t-b commented Aug 27, 2019

@bassosimone

The tests show the same bug if run with ubsan. Bisection, automated and manual, using

set -e

cd build
cmake -DCMAKE_CXX_FLAGS="-fsanitize=undefined -fno-sanitize-recover" ..
cmake --build .
ctest --output-on-failure -R test-deserialization_default

yields as cf8251e (:ambulance: fix compiler errors, 2019-07-14) as buggy and 8d059b9 (Merge pull request #1670 from podsvirov/readme-package-managers-msys2, 2019-07-13) as good.

@t-b
Copy link
Contributor

t-b commented Aug 27, 2019

@bassosimone CI now says that there is an issue, see https://travis-ci.org/nlohmann/json/jobs/577321214#L900 which looks pretty much like what you are seeing.

The PR is #1728.

@bassosimone
Copy link
Author

@t-b correct, thanks a bunch!

@nlohmann nlohmann self-assigned this Sep 10, 2019
@nlohmann nlohmann added solution: proposed fix a fix for the issue has been proposed and waits for confirmation release item: 🔨 further change labels Sep 10, 2019
@nlohmann nlohmann added this to the Release 3.7.1 milestone Sep 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug release item: 🔨 further change solution: proposed fix a fix for the issue has been proposed and waits for confirmation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants