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

Fix bugs reported by asan #5665

Merged
merged 10 commits into from Aug 16, 2019
1 change: 0 additions & 1 deletion osquery/carver/carver.cpp
Expand Up @@ -222,7 +222,6 @@ Status Carver::carve(const boost::filesystem::path& path) {

std::vector<char> inBuff(FLAGS_carver_block_size, 0);
for (size_t i = 0; i < blkCount; i++) {
inBuff.clear();
auto bytesRead = src.read(inBuff.data(), FLAGS_carver_block_size);
if (bytesRead > 0) {
auto bytesWritten = dst.write(inBuff.data(), bytesRead);
Expand Down
2 changes: 1 addition & 1 deletion osquery/filesystem/linux/proc.h
Expand Up @@ -149,7 +149,7 @@ Status procEnumerateProcesses(UserData& user_data,
}

// See #792: std::regex is incomplete until GCC 4.9
const auto& pid = it->path().leaf().string();
const auto pid = it->path().leaf().string();
if (std::atoll(pid.data()) <= 0) {
continue;
}
Expand Down
6 changes: 4 additions & 2 deletions osquery/tables/networking/darwin/routes.cpp
Expand Up @@ -32,7 +32,8 @@ typedef std::pair<int, std::string> RouteType;
typedef std::map<int, std::string> InterfaceMap;
typedef std::vector<struct sockaddr *> AddressMap;

const std::string kDefaultRoute = "0.0.0.0";
constexpr auto kDefaultIPv4Route = "0.0.0.0";
constexpr auto kDefaultIPv6Route = "::";

const std::vector<RouteType> kRouteTypes = {
std::make_pair(RTF_LOCAL, "local"),
Expand Down Expand Up @@ -87,7 +88,8 @@ Status genRoute(const struct rt_msghdr *route,
r["gateway"] = ipAsString(addr_map[RTAX_GATEWAY]);
}

if (r["destination"] == kDefaultRoute) {
if (r["destination"] == kDefaultIPv4Route ||
r["destination"] == kDefaultIPv6Route) {
r["netmask"] = "0";
} else if ((route->rtm_addrs & RTA_NETMASK) == RTA_NETMASK) {
addr_map[RTAX_NETMASK]->sa_family = addr_map[RTAX_DST]->sa_family;
Expand Down
16 changes: 12 additions & 4 deletions osquery/tables/networking/posix/interfaces.cpp
Expand Up @@ -6,6 +6,7 @@
* the LICENSE file found in the root directory of this source tree.
*/

#include <algorithm>
#include <iomanip>
#include <sstream>

Expand Down Expand Up @@ -166,8 +167,12 @@ void genDetailsFromAddr(const struct ifaddrs* addr,
// Get Linux physical properties for the AF_PACKET entry.
int fd = socket(AF_INET, SOCK_DGRAM, 0);
if (fd >= 0) {
struct ifreq ifr;
memcpy(ifr.ifr_name, addr->ifa_name, IFNAMSIZ);
struct ifreq ifr = {};
auto ifa_name_length = strlen(addr->ifa_name);
snprintf(ifr.ifr_name,
std::min<size_t>(ifa_name_length + 1, IFNAMSIZ),
"%s",
addr->ifa_name);
if (ioctl(fd, SIOCGIFMTU, &ifr) >= 0) {
r["mtu"] = BIGINT_FROM_UINT32(ifr.ifr_mtu);
}
Expand Down Expand Up @@ -236,8 +241,11 @@ void genDetailsFromAddr(const struct ifaddrs* addr,
int fd = socket(AF_INET, SOCK_DGRAM, 0);
if (fd >= 0) {
struct ifmediareq ifmr = {};
memcpy(ifmr.ifm_name, addr->ifa_name, sizeof(ifmr.ifm_name));
const std::unique_ptr<int[]> media_list(new int[ifmr.ifm_count]);
auto ifa_name_length = strlen(addr->ifa_name);
snprintf(ifmr.ifm_name,
std::min<size_t>(ifa_name_length + 1, IFNAMSIZ),
"%s",
addr->ifa_name);
if (ioctl(fd, SIOCGIFMEDIA, &ifmr) >= 0) {
if (IFM_TYPE(ifmr.ifm_active) == IFM_ETHER) {
int ifmls = get_linkspeed(IFM_SUBTYPE(ifmr.ifm_active));
Expand Down
15 changes: 12 additions & 3 deletions osquery/tables/system/darwin/processes.cpp
Expand Up @@ -586,7 +586,11 @@ void genMemoryRegion(int pid,
r["pseudo"] = "0";
}

r["offset"] = INTEGER(info.offset);
// Necessary to do an unaligned read without triggering UB
memory_object_offset_t offset;
memcpy(&offset, &info.offset, sizeof(offset));

r["offset"] = INTEGER(offset);
r["device"] = INTEGER(info.object_id);

// Fields not applicable to OS X maps.
Expand Down Expand Up @@ -712,8 +716,13 @@ void genProcessMemoryMap(int pid, QueryData& results, bool exe_only = false) {
mach_msg_type_number_t count = VM_REGION_SUBMAP_INFO_COUNT_64;

vm_size_t size = 0;
status = vm_region_recurse_64(
task, &address, &size, &depth, (vm_region_info_64_t)&info, &count);
status =
vm_region_recurse_64(task,
&address,
&size,
&depth,
reinterpret_cast<vm_region_recurse_info_t>(&info),
&count);

if (status == KERN_INVALID_ADDRESS) {
// Reached the end of the memory map.
Expand Down
18 changes: 1 addition & 17 deletions osquery/tables/system/darwin/smc_keys.cpp
Expand Up @@ -373,23 +373,7 @@ kern_return_t SMCHelper::call(uint32_t selector,
inline uint32_t strtoul(const char *str, size_t size, size_t base) {
uint32_t total = 0;
for (size_t i = 0; i < size; i++) {
if (base == 16) {
total += str[i] << (size - 1 - i) * 8;
} else {
total += (unsigned char)(str[i] << (size - 1 - i) * 8);
}
}
return total;
}

inline float strtof(const char *str, size_t size, size_t e) {
float total = 0;
for (size_t i = 0; i < size; i++) {
if (i == (size - 1)) {
total += (str[i] & 0xff) >> e;
} else {
total += str[i] << (size - 1 - i) * (8 - e);
}
total += (unsigned char)(str[i]) << (size - 1 - i) * 8;
}
return total;
}
Expand Down
2 changes: 2 additions & 0 deletions osquery/tables/system/efi_misc.h
Expand Up @@ -12,6 +12,7 @@
/**
* @brief EFI DevicePath GUIDs, structs, and macros.
*/
#pragma pack(push, 1)
typedef struct {
uint8_t Type;
uint8_t SubType;
Expand All @@ -27,6 +28,7 @@ typedef struct {
uint8_t MBRType;
uint8_t SignatureType;
} HARDDRIVE_DEVICE_PATH;
#pragma pack(pop)

#define EFI_END_ENTIRE_DEVICE_PATH 0xff
#define EFI_END_ENTIRE_DEVICE_PATH_SUBTYPE 0xff
Expand Down
2 changes: 1 addition & 1 deletion osquery/utils/schemer/json/schemer_json.h
Expand Up @@ -73,7 +73,7 @@ Expected<std::string, JsonError> toJson(Type const& value) {
template <typename Type, typename RapidJsonInStream>
ExpectedSuccess<JsonError> fromJson(Type& value, RapidJsonInStream& is) {
auto dom = rapidjson::Document{};
dom.ParseStream(is);
dom.ParseStream<rapidjson::kParseFullPrecisionFlag>(is);
if (dom.HasParseError()) {
return createError(JsonError::Syntax)
<< "Can not parse value of type "
Expand Down
2 changes: 1 addition & 1 deletion osquery/utils/schemer/json/schemer_json_impl.h
Expand Up @@ -174,7 +174,7 @@ class JsonReader final {

template <typename KeyType,
typename ValueType,
typename std::enable_if<std::is_floating_point<ValueType>::value,
typename std::enable_if<std::is_same<ValueType, double>::value,
int>::type = 0>
void copyValueFromJValue(const KeyType& key,
ValueType& value,
Expand Down
6 changes: 3 additions & 3 deletions osquery/utils/schemer/json/tests/schemer_json.cpp
Expand Up @@ -97,7 +97,7 @@ class SecondTestClass {
return second_;
}

float const& getThird() const {
double const& getThird() const {
return third_;
}

Expand All @@ -108,7 +108,7 @@ class SecondTestClass {
private:
std::string first_ = __FILE__;
int second_ = __LINE__;
float third_ = -1;
double third_ = -1;
bool fourth_ = false;
};

Expand Down Expand Up @@ -189,7 +189,7 @@ struct ThirdTestClass {

std::string first = "";
unsigned second = 0u;
float third = 0.;
double third = 0.;
std::int64_t fourth = 0;
};

Expand Down
2 changes: 2 additions & 0 deletions osquery/utils/system/linux/ebpf/perf_output.h
Expand Up @@ -54,11 +54,13 @@ class PerfOutput final {

using MessageBatchType = std::vector<MessageType>;

#pragma pack(push, 1)
struct WrappedMessage {
struct perf_event_header header;
std::uint32_t size;
MessageType msg;
};
#pragma pack(pop)

ExpectedSuccess<PerfOutputError> read(MessageBatchType& dst);

Expand Down
10 changes: 5 additions & 5 deletions osquery/utils/system/linux/ebpf/perf_output_impl.h
Expand Up @@ -213,11 +213,11 @@ consumeWrappedMessagesFromCircularBuffer(ByteType const* data_ptr,
data_tail += wrapped_message.header.size;
offset = data_tail % buffer_size;
} else {
auto wrapped_message =
reinterpret_cast<WrappedMessage const*>(data_ptr + offset);
messages.emplace_back(wrapped_message->msg);
offset += wrapped_message->header.size;
data_tail += wrapped_message->header.size;
WrappedMessage wrapped_message;
memcpy(&wrapped_message, (data_ptr + offset), sizeof(WrappedMessage));
messages.emplace_back(wrapped_message.msg);
offset += wrapped_message.header.size;
data_tail += wrapped_message.header.size;
}
}
return Success{};
Expand Down
2 changes: 1 addition & 1 deletion plugins/logger/tests/tls_logger_tests.cpp
Expand Up @@ -43,7 +43,7 @@ TEST_F(TLSLoggerTests, test_database) {
auto forwarder = std::make_shared<TLSLogForwarder>();
std::string expected = "{\"new_json\": true}";
forwarder->logString(expected);
StatusLogLine status;
StatusLogLine status{};
status.message = "{\"status\": \"bar\"}";
forwarder->logStatus({status});

Expand Down