Uploaded image for project: 'Mesos'
  1. Mesos
  2. MESOS-6424

Possible nullptr dereference in flag loading

    XMLWordPrintableJSON

Details

    • Bug
    • Status: Resolved
    • Major
    • Resolution: Fixed
    • None
    • 1.0.2, 1.1.0
    • stout

    Description

      Coverity reports the following:

      /3rdparty/stout/include/stout/flags/flags.hpp: 375 in flags::FlagsBase::add<mesos::internal::logger::rotate::Flags, std::basic_string<char, std::char_traits<char>, std::allocator<char>>, char [10], mesos::internal::logger::rotate::Flags::Flags()::[lambda(const std::basic_string<char, std::char_traits<char>, std::allocator<char>>&) (instance 1)]>(T2 T1::*, const flags::Name &, const Option<flags::Name> &, const std::basic_string<char, std::char_traits<char>, std::allocator<char>>&, const T3 *, T4)::[lambda(flags::FlagsBase*, const std::basic_string<char, std::char_traits<char>, std::allocator<char>>&) (instance 1)]::operator ()(flags::FlagsBase*, const std::basic_string<char, std::char_traits<char>, std::allocator<char>>&) const()
      369         Flags* flags = dynamic_cast<Flags*>(base);
      370         if (base != nullptr) {
      371           // NOTE: 'fetch' "retrieves" the value if necessary and then
      372           // invokes 'parse'. See 'fetch' for more details.
      373           Try<T1> t = fetch<T1>(value);
      374           if (t.isSome()) {
         CID 1374083:    (FORWARD_NULL)
         Dereferencing null pointer "flags".
      375             flags->*t1 = t.get();
      376           } else {
      377             return Error("Failed to load value '" + value + "': " + t.error());
      378           }
      379         }
      380     
      
      ** CID 1374082:  Null pointer dereferences  (FORWARD_NULL)
      /3rdparty/stout/include/stout/flags/flags.hpp: 375 in flags::FlagsBase::add<mesos::internal::logger::LoggerFlags, Bytes, Megabytes, Option<Error> (*)(const Bytes &)>(T2 T1::*, const flags::Name &, Option<flags::Name>&, const std::basic_string<char, std::char_traits<char>, std::allocator<char>>&, const T3 *, T4)::[lambda(flags::FlagsBase*, const std::basic_string<char, std::char_traits<char>, std::allocator<char>>&) (instance 1)]::operator ()(flags::FlagsBase*, const std::basic_string<char, std::char_traits<char>, std::allocator<char>>&) const()
      
      
      ________________________________________________________________________________________________________
      *** CID 1374082:  Null pointer dereferences  (FORWARD_NULL)
      /3rdparty/stout/include/stout/flags/flags.hpp: 375 in flags::FlagsBase::add<mesos::internal::logger::LoggerFlags, Bytes, Megabytes, Option<Error> (*)(const Bytes &)>(T2 T1::*, const flags::Name &, Option<flags::Name>&, const std::basic_string<char, std::char_traits<char>, std::allocator<char>>&, const T3 *, T4)::[lambda(flags::FlagsBase*, const std::basic_string<char, std::char_traits<char>, std::allocator<char>>&) (instance 1)]::operator ()(flags::FlagsBase*, const std::basic_string<char, std::char_traits<char>, std::allocator<char>>&) const()
      369         Flags* flags = dynamic_cast<Flags*>(base);
      370         if (base != nullptr) {
      371           // NOTE: 'fetch' "retrieves" the value if necessary and then
      372           // invokes 'parse'. See 'fetch' for more details.
      373           Try<T1> t = fetch<T1>(value);
      374           if (t.isSome()) {
         CID 1374082:  Null pointer dereferences  (FORWARD_NULL)
         Dereferencing null pointer "flags".
      375             flags->*t1 = t.get();
      376           } else {
      377             return Error("Failed to load value '" + value + "': " + t.error());
      378           }
      379         }
      380     
      

      The dynamic_cast is needed here if the derived Flags class got intentionally sliced (e.g., to a FlagsBase). Since the base class of the hierarchy (FlagsBase) stores the flags they would not be sliced away; the dynamic_cast here effectively filters out all flags still valid for the Flags used when the Flag was add'ed.

      It seems the intention here was to confirm that the dynamic_cast to Flags* succeeded like is done e.g., in flags.stringify and flags.validate just below.

      AFAICT this code has existed since 2013, but was only reported by coverity recently.

      Attachments

        Activity

          People

            bbannier Benjamin Bannier
            bbannier Benjamin Bannier
            Till Toenshoff Till Toenshoff
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved: