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

Possible nullptr dereference in flag loading

    XMLWordPrintableJSON

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.0.2, 1.1.0
    • Component/s: stout
    • Labels:

      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

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

              Dates

              • Created:
                Updated:
                Resolved: