Safer compile flags

CMakeLists.txt

@@ -2,6 +2,15 @@ cmake_minimum_required(VERSION 2.8.8)
 
 set(CMAKE_C_COMPILER "/usr/bin/clang")
 set(CMAKE_CXX_COMPILER "/usr/bin/clang++")
+set(CXX_COMPILE_FLAGS
+  -Wall
+  -Wextra
+  -Wno-unused-parameter
+  -Wno-missing-field-initializers
+  -Wno-sign-compare
+)
+string(REPLACE ";" " " CXX_COMPILE_FLAGS "${CXX_COMPILE_FLAGS}")
+
 if(APPLE)
   set(APPLE_MIN_ABI "10.9")
   set(CXX_COMPILE_FLAGS "${CXX_COMPILE_FLAGS} -std=c++11 -stdlib=libc++")
@@ -9,7 +18,8 @@ if(APPLE)
   set(OS_WHOLELINK_PRE "-Wl,-force_load")
   set(OS_WHOLELINK_POST "")
   # Special compile flags for Objective-C++
-  set(OBJCXX_COMPILE_FLAGS "-x objective-c++ -fobjc-arc -Wno-c++11-extensions")
+  set(OBJCXX_COMPILE_FLAGS
+    "-x objective-c++ -fobjc-arc -Wno-c++11-extensions -mmacosx-version-min=${APPLE_MIN_ABI}")
 else()
   if(${CMAKE_SYSTEM_NAME} MATCHES "FreeBSD")
     set(FREEBSD TRUE)
@@ -18,7 +28,7 @@ else()
     set(OS_WHOLELINK_POST "")
   else()
     set(LINUX TRUE)
-    set(CXX_COMPILE_FLAGS "-std=c++11")
+    set(CXX_COMPILE_FLAGS "${CXX_COMPILE_FLAGS} -std=c++11")
     set(OS_WHOLELINK_PRE "-Wl,-whole-archive")
     set(OS_WHOLELINK_POST "-Wl,-no-whole-archive")
   endif()
@@ -27,7 +37,7 @@ endif()
 # make debug (environment variable from Makefile)
 if(DEFINED ENV{DEBUG})
   set(CMAKE_BUILD_TYPE "Debug")
-  set(C_COMPILE_FLAGS "${CMAKE_C_FLAGS} -g -DDEBUG -O0")
+  set(C_COMPILE_FLAGS "${CMAKE_C_FLAGS} -g -DDEBUG -O0 -pg")
 else()
   set(C_COMPILE_FLAGS "${CMAKE_C_FLAGS} -O2")
 endif()

Makefile

@@ -23,6 +23,10 @@ debug: .setup
 	cd build/$(BUILD_DIR) && DEBUG=True cmake ../../ && \
 		$(MAKE) --no-print-directory $(MAKEFLAGS)
 
+test_debug: .setup
+	cd build/$(BUILD_DIR)/sdk && DEBUG=True cmake ../../../ && \
+	  $(MAKE) test --no-print-directory $(MAKEFLAGS)	
+
 analyze: .setup
 	cd build/$(BUILD_DIR) && ANALYZE=True cmake ../../ && \
 	  $(MAKE) --no-print-directory $(MAKEFLAGS)

include/osquery/registry.h

@@ -111,7 +111,7 @@ class Plugin {
   /// The plugin may perform some tear down, release, not required.
   virtual void tearDown() {}
   /// The plugin may publish route info (other than registry type and name).
-  virtual RouteInfo routeInfo() {
+  virtual RouteInfo routeInfo() const {
     RouteInfo info;
     return info;
   }
@@ -255,13 +255,11 @@ class RegistryHelper : public RegistryHelperCore {
       return Status(1, "Duplicate registry item exists: " + item_name);
     }
 
-    // Run the item's constructor, the setUp call will happen later.
-    auto item = (RegistryType*)new Item();
+    // Cast the specific registry-type derived item as the API type of the
+    // registry used when created using the registry factory.
+    std::shared_ptr<RegistryType> item((RegistryType*)new Item());
     item->setName(item_name);
-    // Cast the specific registry-type derived item as the API typ the registry
-    // used when it was created using the registry factory.
-    std::shared_ptr<RegistryType> shared_item(item);
-    items_[item_name] = shared_item;
+    items_[item_name] = item;
     return Status(0, "OK");
   }
 
@@ -274,11 +272,11 @@ class RegistryHelper : public RegistryHelperCore {
    * @param item_name An identifier for this registry plugin.
    * @return A std::shared_ptr of type RegistryType.
    */
-  RegistryTypeRef get(const std::string& item_name) {
+  RegistryTypeRef get(const std::string& item_name) const {
     return std::dynamic_pointer_cast<RegistryType>(items_.at(item_name));
   }
 
-  const std::map<std::string, RegistryTypeRef> all() {
+  const std::map<std::string, RegistryTypeRef> all() const {
     std::map<std::string, RegistryTypeRef> ditems;
     for (const auto& item : items_) {
       ditems[item.first] = std::dynamic_pointer_cast<RegistryType>(item.second);
@@ -326,10 +324,9 @@ class RegistryFactory : private boost::noncopyable {
       return 0;
     }
 
-    auto registry = (PluginRegistryHelper*)new RegistryHelper<Type>(auto_setup);
+    PluginRegistryHelperRef registry((PluginRegistryHelper*)new RegistryHelper<Type>(auto_setup));
     registry->setName(registry_name);
-    PluginRegistryHelperRef shared_registry(registry);
-    instance().registries_[registry_name] = shared_registry;
+    instance().registries_[registry_name] = registry;
     return 0;
   }
 

include/osquery/tables.h

@@ -124,7 +124,7 @@ struct ConstraintList {
    * @param expr a SQL type expression of the column literal type to check.
    * @return If the expression matched all constraints.
    */
-  bool matches(const std::string& expr);
+  bool matches(const std::string& expr) const;
 
   /**
    * @brief Check if an expression matches the query constraints.
@@ -137,7 +137,7 @@ struct ConstraintList {
    * @return If the expression matched all constraints.
    */
   template <typename T>
-  bool matches(const T& expr) {
+  bool matches(const T& expr) const {
     return matches(TEXT(expr));
   }
 
@@ -150,7 +150,7 @@ struct ConstraintList {
    *
    * @return true if any constraint exists.
    */
-  bool exists() { return (constraints_.size() > 0); }
+  bool exists() const { return (constraints_.size() > 0); }
 
   /**
    * @brief Check if a constrait exist AND matches the type expression.
@@ -161,7 +161,7 @@ struct ConstraintList {
    * @return true if any constraint exists AND matches the type expression.
    */
   template <typename T>
-  bool existsAndMatches(const T& expr) {
+  bool existsAndMatches(const T& expr) const {
     return (exists() && matches(expr));
   }
 
@@ -176,7 +176,7 @@ struct ConstraintList {
    * @return true if constraint is missing or matches the type expression.
    */
   template <typename T>
-  bool notExistsOrMatches(const T& expr) {
+  bool notExistsOrMatches(const T& expr) const {
     return (!exists() || matches(expr));
   }
 
@@ -184,7 +184,7 @@ struct ConstraintList {
    * @brief Helper templated function for ConstraintList::matches.
    */
   template <typename T>
-  bool literal_matches(const T& base_expr);
+  bool literal_matches(const T& base_expr) const;
 
   /**
    * @brief Get all expressions for a given ConstraintOperator.
@@ -195,10 +195,10 @@ struct ConstraintList {
    * @param op the ConstraintOperator.
    * @return A list of TEXT%-represented types matching the operator.
    */
-  std::set<std::string> getAll(ConstraintOperator op);
+  std::set<std::string> getAll(ConstraintOperator op) const;
 
   template<typename T>
-  std::set<T> getAll(ConstraintOperator op) {
+  std::set<T> getAll(ConstraintOperator op) const {
     std::set<T> literal_matches;
     auto matches = getAll(op);
     for (const auto& match : matches) {
@@ -273,9 +273,9 @@ typedef struct Constraint Constraint;
 class TablePlugin : public Plugin {
  protected:
   /// Helper method to generate the virtual table CREATE statement.
-  virtual std::string statement();
-  virtual std::string columnDefinition();
-  virtual TableColumns columns() {
+  virtual std::string statement() const;
+  virtual std::string columnDefinition() const;
+  virtual TableColumns columns() const {
     TableColumns columns;
     return columns;
   }

osquery/core/tables.cpp

@@ -17,7 +17,7 @@ namespace osquery {
 namespace tables {
 
 
-bool ConstraintList::matches(const std::string& expr) {
+bool ConstraintList::matches(const std::string& expr) const {
   // Support each SQL affinity type casting.
   if (affinity == "TEXT") {
     return literal_matches<TEXT_LITERAL>(expr);
@@ -37,7 +37,7 @@ bool ConstraintList::matches(const std::string& expr) {
 }
 
 template <typename T>
-bool ConstraintList::literal_matches(const T& base_expr) {
+bool ConstraintList::literal_matches(const T& base_expr) const {
   bool aggregate = true;
   for (size_t i = 0; i < constraints_.size(); ++i) {
     T constraint_expr = AS_LITERAL(T, constraints_[i].expr);
@@ -63,7 +63,7 @@ bool ConstraintList::literal_matches(const T& base_expr) {
   return true;
 }
 
-std::set<std::string> ConstraintList::getAll(ConstraintOperator op) {
+std::set<std::string> ConstraintList::getAll(ConstraintOperator op) const {
   std::set<std::string> set;
   for (size_t i = 0; i < constraints_.size(); ++i) {
     if (constraints_[i].op == op) {
@@ -122,9 +122,7 @@ void TablePlugin::setRequestFromContext(const QueryContext& context,
 
 void TablePlugin::setResponseFromQueryData(const QueryData& data,
                                            PluginResponse& response) {
-  for (const auto& row : data) {
-    response.push_back(row);
-  }
+  response = std::move(data);
 }
 
 void TablePlugin::setContextFromRequest(const PluginRequest& request,
@@ -170,7 +168,7 @@ Status TablePlugin::call(const PluginRequest& request,
   } else if (request.at("action") == "columns") {
     // "columns" returns a PluginRequest filled with column information
     // such as name and type.
-    auto column_list = columns();
+    const auto& column_list = columns();
     for (const auto& column : column_list) {
       response.push_back({{"name", column.first}, {"type", column.second}});
     }
@@ -183,20 +181,19 @@ Status TablePlugin::call(const PluginRequest& request,
   return Status(0, "OK");
 }
 
-std::string TablePlugin::columnDefinition() {
+std::string TablePlugin::columnDefinition() const {
   const auto& column_list = columns();
   std::string statement = "(";
   for (size_t i = 0; i < column_list.size(); ++i) {
-    statement += column_list[i].first + " " + column_list.at(i).second;
+    statement += column_list.at(i).first + " " + column_list.at(i).second;
     if (i < column_list.size() - 1) {
       statement += ", ";
     }
   }
-  statement += ")";
-  return statement;
+  return statement += ")";
 }
 
-std::string TablePlugin::statement() {
+std::string TablePlugin::statement() const {
   return "CREATE TABLE " + name_ + columnDefinition();
 }
 

osquery/events/darwin/fsevents.cpp

@@ -164,7 +164,7 @@ void FSEventsEventPublisher::Callback(
 
 bool FSEventsEventPublisher::shouldFire(
     const FSEventsSubscriptionContextRef& mc,
-    const FSEventsEventContextRef& ec) {
+    const FSEventsEventContextRef& ec) const {
   ssize_t found = ec->path.find(mc->path);
   if (found != 0) {
     return false;

osquery/events/darwin/fsevents.h

@@ -92,7 +92,7 @@ class FSEventsEventPublisher
   }
 
   bool shouldFire(const FSEventsSubscriptionContextRef& mc,
-                  const FSEventsEventContextRef& ec);
+                  const FSEventsEventContextRef& ec) const;
 
  private:
   // Restart the run loop.

osquery/events/darwin/iokit_hid.cpp

@@ -136,7 +136,7 @@ void IOKitHIDEventPublisher::InputValueCallback(void *context,
 
 bool IOKitHIDEventPublisher::shouldFire(
     const IOKitHIDSubscriptionContextRef &sc,
-    const IOKitHIDEventContextRef &ec) {
+    const IOKitHIDEventContextRef &ec) const {
   if (sc->values) {
     // See InputValueCallback
     return false;

osquery/events/darwin/iokit_hid.h

@@ -114,7 +114,7 @@ class IOKitHIDEventPublisher
   }
 
   bool shouldFire(const IOKitHIDSubscriptionContextRef &mc,
-                  const IOKitHIDEventContextRef &ec);
+                  const IOKitHIDEventContextRef &ec) const;
 
  public:
   /**

osquery/events/darwin/scnetwork.cpp

@@ -41,7 +41,7 @@ void SCNetworkEventPublisher::Callback(const SCNetworkReachabilityRef target,
 
 bool SCNetworkEventPublisher::shouldFire(
     const SCNetworkSubscriptionContextRef& sc,
-    const SCNetworkEventContextRef& ec) {
+    const SCNetworkEventContextRef& ec) const {
   // Only fire the event for the subscription context it matched.
   return (sc == ec->subscription);
 }

osquery/events/darwin/scnetwork.h

@@ -71,7 +71,7 @@ class SCNetworkEventPublisher
  public:
   SCNetworkEventPublisher() : EventPublisher(), run_loop_(nullptr) {}
   bool shouldFire(const SCNetworkSubscriptionContextRef& sc,
-                  const SCNetworkEventContextRef& ec);
+                  const SCNetworkEventContextRef& ec) const;
 
  private:
   // Restart the run loop by calling configure.

osquery/events/linux/inotify.cpp

@@ -141,7 +141,7 @@ INotifyEventContextRef INotifyEventPublisher::createEventContextFrom(
 }
 
 bool INotifyEventPublisher::shouldFire(const INotifySubscriptionContextRef& sc,
-                                       const INotifyEventContextRef& ec) {
+                                       const INotifyEventContextRef& ec) const {
   if (!sc->recursive && sc->path != ec->path) {
     // Monitored path is not recursive and path is not an exact match.
     return false;

osquery/events/linux/inotify.h

@@ -122,7 +122,7 @@ class INotifyEventPublisher
   bool removeMonitor(int watch, bool force = false);
   /// Given a SubscriptionContext and INotifyEventContext match path and action.
   bool shouldFire(const INotifySubscriptionContextRef& mc,
-                  const INotifyEventContextRef& ec);
+                  const INotifyEventContextRef& ec) const;
   /// Get the INotify file descriptor.
   int getHandle() { return inotify_handle_; }
   /// Get the number of actual INotify active descriptors.

osquery/events/linux/udev.cpp

@@ -138,7 +138,7 @@ UdevEventContextRef UdevEventPublisher::createEventContextFrom(
 }
 
 bool UdevEventPublisher::shouldFire(const UdevSubscriptionContextRef& sc,
-                                    const UdevEventContextRef& ec) {
+                                    const UdevEventContextRef& ec) const {
   if (sc->action != UDEV_EVENT_ACTION_ALL) {
     if (sc->action != ec->action) {
       return false;

osquery/events/linux/udev.h

@@ -113,7 +113,7 @@ class UdevEventPublisher
  private:
   /// Check subscription details.
   bool shouldFire(const UdevSubscriptionContextRef& mc,
-                  const UdevEventContextRef& ec);
+                  const UdevEventContextRef& ec) const;
   /// Helper function to create an EventContext using a udev_device pointer.
   UdevEventContextRef createEventContextFrom(struct udev_device* device);
 };

osquery/main/run.cpp

@@ -49,13 +49,14 @@ int main(int argc, char* argv[]) {
     if (!status.ok()) {
       fprintf(stderr, "Query failed: %d\n", status.getCode());
       break;
-    } else {
-      if (FLAGS_delay != 0) {
-        ::sleep(FLAGS_delay);
-      }
     }
   }
 
+  if (FLAGS_delay != 0) {
+    ::sleep(FLAGS_delay);
+  }
+
+
   // Instead of calling "shutdownOsquery" force the EF to join its threads.
   osquery::EventFactory::end(true);
   __GFLAGS_NAMESPACE::ShutDownCommandLineFlags();

osquery/registry/registry.cpp

@@ -60,7 +60,7 @@ Status RegistryHelperCore::call(const std::string& item_name,
                                 const PluginRequest& request,
                                 PluginResponse& response) {
   if (items_.count(item_name) > 0) {
-    return items_[item_name]->call(request, response);
+    return items_.at(item_name)->call(request, response);
   }
   return Status(1, "Cannot call registry item: " + item_name);
 }
@@ -206,7 +206,7 @@ Status RegistryFactory::call(const std::string& registry_name,
   if (instance().registries_.count(registry_name) == 0) {
     return Status(1, "Unknown registry: " + registry_name);
   }
-  return instance().registries_[registry_name]->call(
+  return instance().registries_.at(registry_name)->call(
       item_name, request, response);
 }
 

osquery/registry/registry_tests.cpp

@@ -154,7 +154,7 @@ class WidgetPlugin : public Plugin {
   /// to parse and format. BUT a plugin/registry item can also fill this
   /// information in if the plugin type/registry type exposes routeInfo as
   /// a virtual method.
-  RouteInfo routeInfo() {
+  RouteInfo routeInfo() const {
     RouteInfo info;
     info["name"] = name_;
     return info;
@@ -162,7 +162,7 @@ class WidgetPlugin : public Plugin {
 
   /// Plugin types should contain generic request/response formatters and
   /// decorators.
-  std::string secretPower(const PluginRequest& request) {
+  std::string secretPower(const PluginRequest& request) const {
     if (request.count("secret_power") > 0) {
       return request.at("secret_power");
     }
@@ -183,8 +183,12 @@ Status SpecialWidget::call(const PluginRequest& request,
   return Status(0, "OK");
 }
 
+#define UNUSED(x) (void)(x)
+
 TEST_F(RegistryTests, test_registry_api) {
   auto AutoWidgetRegistry = TestCoreRegistry::create<WidgetPlugin>("widgets");
+  UNUSED(AutoWidgetRegistry);
+
   TestCoreRegistry::add<SpecialWidget>("widgets", "special");
 
   // Test route info propogation, from item to registry, to broadcast.

osquery/sql/sql_tests.cpp

@@ -37,7 +37,7 @@ TEST_F(SQLTests, test_raw_access) {
 
 class TestTable : public tables::TablePlugin {
  private:
-  tables::TableColumns columns() {
+  tables::TableColumns columns() const {
     return {{"test_int", "INTEGER"}, {"test_text", "TEXT"}};
   }
 

osquery/sql/virtual_table_tests.cpp

@@ -24,7 +24,7 @@ class VirtualTableTests : public testing::Test {};
 // sample plugin used on tests
 class sampleTablePlugin : public TablePlugin {
  private:
-  TableColumns columns() {
+  TableColumns columns() const {
     return {
         {"foo", "INTEGER"}, {"bar", "TEXT"},
     };

osquery/tables/system/darwin/iokit_registry.cpp

@@ -28,7 +28,7 @@ void genIOKitDevice(const io_service_t& device,
   IORegistryEntryCreateCFProperties(
       device, &details, kCFAllocatorDefault, kNilOptions);
 
-  io_name_t name, location, device_class;
+  io_name_t name, device_class;
   auto kr = IORegistryEntryGetName(device, name);
   if (kr == KERN_SUCCESS) {
     r["name"] = std::string(name);

osquery/tables/system/darwin/kernel_info.cpp

@@ -58,13 +58,13 @@ std::string getCanonicalEfiDevicePath(const CFDataRef& data) {
       } else if (node->SubType == MEDIA_HARDDRIVE_DP) {
         // Extract the device UUID to later join with block devices.
         auto uuid = ((const HARDDRIVE_DEVICE_PATH*)node)->Signature;
-        boost::uuids::uuid hdd_signature = {
+        boost::uuids::uuid hdd_signature = {{
           uuid[3], uuid[2], uuid[1], uuid[0],
           uuid[5], uuid[4],
           uuid[7], uuid[6],
           uuid[8], uuid[9],
           uuid[10], uuid[11], uuid[12], uuid[13], uuid[14], uuid[15],
-        };
+        }};
         path += boost::to_upper_copy(boost::uuids::to_string(hdd_signature));
       }
     }

osquery/tables/system/darwin/nvram.cpp

@@ -61,10 +61,7 @@ void genVariable(const void *key, const void *value, void *results) {
 QueryData genNVRAM(QueryContext &context) {
   QueryData results;
 
-  kern_return_t status;
   mach_port_t master_port;
-  io_registry_entry_t options_ref;
-
   auto kr = IOMasterPort(bootstrap_port, &master_port);
   if (kr != KERN_SUCCESS) {
     VLOG(1) << "Could not get the IOMaster port";

osquery/tables/system/darwin/xattr.cpp

@@ -153,8 +153,7 @@ void getFileData(Row& r,
                  const std::string& directory) {
   r["path"] = path;
   r["directory"] = directory;
-  int ret;
-  int buf_len;
+
   struct XAttrAttribute x_att =
       getAttribute(path, "com.apple.metadata:kMDItemWhereFroms");
   parseWhereFromData(r, x_att);

osquery/tables/system/linux/acpi_tables.cpp

@@ -32,7 +32,7 @@ void genACPITable(const std::string& table, QueryData& results) {
     status = osquery::listFilesInDirectory(table_path, child_tables);
     if (status.ok()) {
       for (const auto& child_table : child_tables) {
-        genACPITable(table, results);
+        genACPITable(child_table, results);
       }
     }
 

osquery/tables/system/linux/apt_sources.cpp

@@ -46,7 +46,7 @@ bool isFieldOkay(const char* fieldValue) {
 }
 
 void extractAptSourceInfo(pkgCache::PkgFileIterator src,
-                          const struct pkgIndexFile* pkgIndex,
+                          const pkgIndexFile* pkgIndex,
                           QueryData& results) {
   Row r;
 

osquery/tables/system/linux/block_devices.cpp

@@ -70,33 +70,33 @@ static void fillRow(struct udev_device *dev, Row &r) {
 
 QueryData genBlockDevs(QueryContext &context) {
   QueryData results;
-  struct udev *udev;
-  struct udev_enumerate *enumerate;
-  struct udev_list_entry *devices, *dev_list_entry;
-  struct udev_device *dev, *parent;
 
-  if ((udev = udev_new())) {
-    enumerate = udev_enumerate_new(udev);
-    udev_enumerate_add_match_subsystem(enumerate, "block");
-    udev_enumerate_scan_devices(enumerate);
-    devices = udev_enumerate_get_list_entry(enumerate);
-    udev_list_entry_foreach(dev_list_entry, devices) {
-      const char *path;
-      Row r;
-
-      path = udev_list_entry_get_name(dev_list_entry);
-      dev = udev_device_new_from_syspath(udev, path);
-
-      fillRow(dev, r);
-
-      results.push_back(r);
-      udev_device_unref(dev);
-    }
-
-    udev_enumerate_unref(enumerate);
-    udev_unref(udev);
+  struct udev *udev = udev_new();
+  if (udev == nullptr) {
+    return {};
   }
 
+  struct udev_enumerate *enumerate = udev_enumerate_new(udev);
+  udev_enumerate_add_match_subsystem(enumerate, "block");
+  udev_enumerate_scan_devices(enumerate);
+
+  struct udev_list_entry *devices, *dev_list_entry;
+  devices = udev_enumerate_get_list_entry(enumerate);
+  udev_list_entry_foreach(dev_list_entry, devices) {
+    Row r;
+
+    const char *path = udev_list_entry_get_name(dev_list_entry);
+    struct udev_device *dev = udev_device_new_from_syspath(udev, path);
+
+    fillRow(dev, r);
+
+    results.push_back(r);
+    udev_device_unref(dev);
+  }
+
+  udev_enumerate_unref(enumerate);
+  udev_unref(udev);
+
   return results;
 }
 }

osquery/tables/templates/default.cpp.in

@@ -29,7 +29,7 @@ class {{class_name}} {
 
 class {{table_name_cc}}TablePlugin : public TablePlugin {
  private:
-  TableColumns columns() {
+  TableColumns columns() const {
     return {
 {% for column in schema %}\
       {"{{column.name}}", "{{column.type.affinity}}"}\