diff --git a/Makefile b/Makefile index 351e03c..4870df4 100644 --- a/Makefile +++ b/Makefile @@ -110,6 +110,8 @@ $(info Building $(VERSION)) FUZZFLAGS ?= -DFUZZING=false CXXFLAGS ?= $(DEBUG_FLAGS) $(FUZZFLAGS) -fPIC -std=c++11 -Wall -Werror=format-security +# Additional fedora rpm package build flags +# -O2 -flto=auto -ffat-lto-objects -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fstack-protector-strong -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection CXXFLAGS += -I$(BUILD) LDFLAGS ?= $(DEBUG_LDFLAGS) diff --git a/src/dvparser.cc b/src/dvparser.cc index 0307206..945f3f3 100644 --- a/src/dvparser.cc +++ b/src/dvparser.cc @@ -443,7 +443,7 @@ bool parseDV(Telegram *t, } string format_string = bin2hex(format_bytes); - uint16_t hash = crc16_EN13757(&format_bytes[0], format_bytes.size()); + uint16_t hash = crc16_EN13757(safeButUnsafeVectorPtr(format_bytes), format_bytes.size()); if (data_has_difvifs) { if (hash_to_format_.count(hash) == 0) { diff --git a/src/lora_iu880b.cc b/src/lora_iu880b.cc index 037b353..32c6c55 100644 --- a/src/lora_iu880b.cc +++ b/src/lora_iu880b.cc @@ -93,7 +93,7 @@ struct Firmware_IU880B major = bytes[i++]; build_count = bytes[i] | bytes[i+1]; i+=2; - image = string(&bytes[0]+i, &bytes[0]+bytes.size()); + image = string(&bytes[0]+i, &bytes[0]+bytes.size()); // Ptr from &[0] is ok since size is > 0 return true; } }; @@ -511,7 +511,7 @@ static void buildRequest(int endpoint_id, int msg_id, vector& body, vecto request.push_back(msg_id); request.insert(request.end(), body.begin(), body.end()); - uint16_t crc = ~crc16_CCITT(&request[0], request.size()); + uint16_t crc = ~crc16_CCITT(&request[0], request.size()); // Safe to use &[0] since request size > 0 request.push_back(crc & 0xff); request.push_back(crc >> 8); diff --git a/src/main.cc b/src/main.cc index a11d7b5..80cb3f5 100644 --- a/src/main.cc +++ b/src/main.cc @@ -304,10 +304,10 @@ void list_meters(Configuration *config) LIST_OF_METERS #undef X - for (DriverInfo &di : allRegisteredDrivers()) + for (DriverInfo *di : allDrivers()) { - string mname = di.name().str(); - const char *info = toString(di.type()); + string mname = di->name().str(); + const char *info = toString(di->type()); if (config->list_meters_search == "" || \ stringFoundCaseIgnored(info, config->list_meters_search) || \ diff --git a/src/meters.cc b/src/meters.cc index 1301b1e..4704e05 100644 --- a/src/meters.cc +++ b/src/meters.cc @@ -32,8 +32,40 @@ #include #include -map all_registered_drivers_; -vector all_registered_drivers_list_; +map *registered_drivers_ = NULL; +vector *registered_drivers_list_ = NULL; + +void verifyDriverLookupCreated() +{ + if (registered_drivers_ == NULL) + { + registered_drivers_ = new map; + } + if (registered_drivers_list_ == NULL) + { + registered_drivers_list_ = new vector; + } +} + +DriverInfo *lookupDriver(string name) +{ + verifyDriverLookupCreated(); + if (registered_drivers_->count(name) == 0) return NULL; + return &(*registered_drivers_)[name]; +} + +vector &allDrivers() +{ + return *registered_drivers_list_; +} + +void addRegisteredDriver(DriverInfo di) +{ + verifyDriverLookupCreated(); + (*registered_drivers_)[di.name().str()] = di; + // The list elements points into the map. + (*registered_drivers_list_).push_back(lookupDriver(di.name().str())); +} bool DriverInfo::detect(uint16_t mfct, uchar type, uchar version) { @@ -68,26 +100,25 @@ bool registerDriver(function setup) DriverInfo di; setup(di); - // Check that the driver name is unique. - assert(all_registered_drivers_.count(di.name().str()) == 0); + // Check that the driver name has not been registered before! + assert(lookupDriver(di.name().str()) == NULL); // Check that no other driver also triggers on the same detection values. for (auto &d : di.detect()) { - for (auto &p : all_registered_drivers_) + for (DriverInfo *p : allDrivers()) { - bool foo = p.second.detect(d.mfct, d.type, d.version); + bool foo = p->detect(d.mfct, d.type, d.version); if (foo) { error("Internal error: driver %s tried to register the same auto detect combo as driver %s alread has taken!\n", - di.name().str().c_str(), p.second.name().str().c_str()); + di.name().str().c_str(), p->name().str().c_str()); } } } // Everything looks, good install this driver. - all_registered_drivers_[di.name().str()] = di; - all_registered_drivers_list_.push_back(di); + addRegisteredDriver(di); // This code is invoked from the static initializers of DriverInfos when starting // wmbusmeters. Thus we do not yet know if the user has supplied --debug or similar setting. @@ -98,12 +129,14 @@ bool registerDriver(function setup) bool lookupDriverInfo(string& driver, DriverInfo *out_di) { - if (all_registered_drivers_.count(driver) == 0) + DriverInfo *di = lookupDriver(driver); + if (di == NULL) { return false; } - *out_di = all_registered_drivers_[driver]; + *out_di = *di; + return true; } @@ -444,9 +477,9 @@ LIST_OF_METERS { string best_driver = ""; - for (DriverInfo ndr : all_registered_drivers_list_) + for (DriverInfo *ndr : allDrivers()) { - string driver_name = toString(ndr); + string driver_name = toString(*ndr); if (only != "") { if (driver_name != only) continue; @@ -493,7 +526,7 @@ LIST_OF_METERS { *best_understood = u; *best_length = l; - best_driver = ndr.name().str(); + best_driver = ndr->name().str(); if (analyze_verbose_ && only == "") printf("(verbose) new best so far: %s %02d/%02d\n", best_driver.c_str(), u, l); } } @@ -1242,15 +1275,15 @@ LIST_OF_METERS return false; } - if (all_registered_drivers_.count(dn.str()) == 0) return false; + DriverInfo *di = lookupDriver(dn.str()); - DriverInfo& di = all_registered_drivers_[dn.str()]; + if (di == NULL) return false; // Return true for MBUS,S2,C2,T2 meters. Currently only mbus is implemented. - return di.linkModes().has(LinkMode::MBUS) || - di.linkModes().has(LinkMode::C2) || - di.linkModes().has(LinkMode::T2) || - di.linkModes().has(LinkMode::S2); + return di->linkModes().has(LinkMode::MBUS) || + di->linkModes().has(LinkMode::C2) || + di->linkModes().has(LinkMode::T2) || + di->linkModes().has(LinkMode::S2); } const char *toString(MeterType type) @@ -2079,11 +2112,11 @@ void detectMeterDrivers(int manufacturer, int media, int version, vector METER_DETECTION #undef X - for (auto &p : all_registered_drivers_) + for (DriverInfo *p : allDrivers()) { - if (p.second.detect(manufacturer, media, version)) + if (p->detect(manufacturer, media, version)) { - drivers->push_back(p.second.name().str()); + drivers->push_back(p->name().str()); } } } @@ -2094,9 +2127,9 @@ bool isMeterDriverValid(MeterDriver type, int manufacturer, int media, int versi METER_DETECTION #undef X - for (auto &p : all_registered_drivers_) + for (DriverInfo *p : allDrivers()) { - if (p.second.detect(manufacturer, media, version)) + if (p->detect(manufacturer, media, version)) { return true; } @@ -2115,9 +2148,9 @@ METER_DETECTION #undef X } - for (auto &p : all_registered_drivers_) + for (DriverInfo *p : allDrivers()) { - if (p.first == driver_name && p.second.isValidMedia(media)) + if (p->name().str() == driver_name && p->isValidMedia(media)) { return true; } @@ -2126,11 +2159,6 @@ METER_DETECTION return false; } -vector& allRegisteredDrivers() -{ - return all_registered_drivers_list_; -} - DriverInfo pickMeterDriver(Telegram *t) { int manufacturer = t->dll_mfct; @@ -2148,11 +2176,11 @@ DriverInfo pickMeterDriver(Telegram *t) METER_DETECTION #undef X - for (auto &p : all_registered_drivers_) + for (DriverInfo *p : allDrivers()) { - if (p.second.detect(manufacturer, media, version)) + if (p->detect(manufacturer, media, version)) { - return p.second; + return *p; } } @@ -2165,15 +2193,16 @@ shared_ptr createMeter(MeterInfo *mi) const char *keymsg = (mi->key[0] == 0) ? "not-encrypted" : "encrypted"; - if (all_registered_drivers_.count(mi->driver_name.str()) != 0) + DriverInfo *di = lookupDriver(mi->driver_name.str()); + + if (di != NULL) { - DriverInfo& di = all_registered_drivers_[mi->driver_name.str()]; - shared_ptr newm = di.construct(*mi); + shared_ptr newm = di->construct(*mi); newm->addConversions(mi->conversions); newm->setPollInterval(mi->poll_interval); verbose("(meter) created %s %s %s %s\n", mi->name.c_str(), - di.name().str().c_str(), + di->name().str().c_str(), mi->idsc.c_str(), keymsg); return newm; diff --git a/src/meters.h b/src/meters.h index f0f15ff..044575d 100644 --- a/src/meters.h +++ b/src/meters.h @@ -281,7 +281,7 @@ DriverInfo pickMeterDriver(Telegram *t); // Return true for mbus and S2/C2/T2 drivers. bool driverNeedsPolling(MeterDriver driver, DriverName& dn); -vector& allRegisteredDrivers(); +vector& allDrivers(); //////////////////////////////////////////////////////////////////////////////////////////////////////////// diff --git a/src/testinternals.cc b/src/testinternals.cc index 96e1fda..1615234 100644 --- a/src/testinternals.cc +++ b/src/testinternals.cc @@ -535,7 +535,9 @@ void test_kdf() hex2bin("2b7e151628aed2a6abf7158809cf4f3c", &key); mac.resize(16); - AES_CMAC(&key[0], &input[0], 0, &mac[0]); + AES_CMAC(safeButUnsafeVectorPtr(key), + safeButUnsafeVectorPtr(input), 0, + safeButUnsafeVectorPtr(mac)); string s = bin2hex(mac); string ex = "BB1D6929E95937287FA37D129B756746"; if (s != ex) @@ -546,7 +548,9 @@ void test_kdf() input.clear(); hex2bin("6bc1bee22e409f96e93d7e117393172a", &input); - AES_CMAC(&key[0], &input[0], 16, &mac[0]); + AES_CMAC(safeButUnsafeVectorPtr(key), + safeButUnsafeVectorPtr(input), 16, + safeButUnsafeVectorPtr(mac)); s = bin2hex(mac); ex = "070A16B46B4D4144F79BDD9DD04A287C"; @@ -1066,14 +1070,14 @@ void test_aes() debug("(aes) input: \"%s\"\n", poe.c_str()); uchar out[sizeof(in)]; - AES_CBC_encrypt_buffer(out, in, sizeof(in), &key[0], iv); + AES_CBC_encrypt_buffer(out, in, sizeof(in), safeButUnsafeVectorPtr(key), iv); vector outv(out, out+sizeof(out)); string s = bin2hex(outv); debug("(aes) encrypted: \"%s\"\n", s.c_str()); uchar back[sizeof(in)]; - AES_CBC_decrypt_buffer(back, out, sizeof(in), &key[0], iv); + AES_CBC_decrypt_buffer(back, out, sizeof(in), safeButUnsafeVectorPtr(key), iv); string b = string(back, back+sizeof(back)); debug("(aes) decrypted: \"%s\"\n", b.c_str()); @@ -1083,8 +1087,8 @@ void test_aes() printf("ERROR! aes with IV encrypt decrypt failed!\n"); } - AES_ECB_encrypt(in, &key[0], out, sizeof(in)); - AES_ECB_decrypt(out, &key[0], back, sizeof(in)); + AES_ECB_encrypt(in, safeButUnsafeVectorPtr(key), out, sizeof(in)); + AES_ECB_decrypt(out, safeButUnsafeVectorPtr(key), back, sizeof(in)); if (memcmp(back, in, sizeof(in))) { diff --git a/src/translatebits.cc b/src/translatebits.cc index 0871dfa..1e9787d 100644 --- a/src/translatebits.cc +++ b/src/translatebits.cc @@ -179,7 +179,7 @@ string Lookup::translate(uint64_t bits) handleRule(r, s, bits); } - while (s.back() == ' ') s.pop_back(); + while (s.size() > 0 && s.back() == ' ') s.pop_back(); return s; } diff --git a/src/units.cc b/src/units.cc index 26c24f4..d2c762e 100644 --- a/src/units.cc +++ b/src/units.cc @@ -179,7 +179,7 @@ string valueToString(double v, Unit u) return "null"; } string s = to_string(v); - while (s.back() == '0') s.pop_back(); + while (s.size() > 0 && s.back() == '0') s.pop_back(); if (s.back() == '.') { s.pop_back(); if (s.length() == 0) return "0"; diff --git a/src/util.cc b/src/util.cc index e03c9d6..d1d16c2 100644 --- a/src/util.cc +++ b/src/util.cc @@ -2277,7 +2277,13 @@ string sortStatusString(const string &a) result += s+" "; } - while (result.back() == ' ') result.pop_back(); + while (result.size() > 0 && result.back() == ' ') result.pop_back(); return result; } + +uchar *safeButUnsafeVectorPtr(std::vector &v) +{ + if (v.size() == 0) return NULL; + return &v[0]; +} diff --git a/src/util.h b/src/util.h index b8cbdc1..db4a37d 100644 --- a/src/util.h +++ b/src/util.h @@ -272,6 +272,12 @@ std::string joinStatusStrings(const std::string &a, const std::string &b); // Also identical flags are merged: "BETA ALFA ALFA" --> "ALFA BETA" std::string sortStatusString(const std::string &a); +// If a vector is empty, then there will be an assert (with some compiler flags) if we use &v[0] +// even if we do not intend to actually use the pointer to uchars! +// So provide safeVectorPtr which will return NULL instead of assert-crashing. +uchar *safeButUnsafeVectorPtr(std::vector &v); + + #ifndef FUZZING #define FUZZING false #endif diff --git a/src/wmbus.cc b/src/wmbus.cc index 8092db1..ff13f0f 100644 --- a/src/wmbus.cc +++ b/src/wmbus.cc @@ -372,7 +372,7 @@ deque seen_telegrams; bool seen_this_telegram_before(vector &frame) { SHA256_HASH hash; - Sha256Calculate(&frame[0], frame.size(), &hash); + Sha256Calculate(safeButUnsafeVectorPtr(frame), frame.size(), &hash); auto i = std::find(seen_telegrams.begin(), seen_telegrams.end(), hash); @@ -1365,7 +1365,9 @@ bool Telegram::parseTPLConfig(std::vector::iterator &pos) debug("(wmbus) no key, thus cannot execute kdf.\n"); return false; } - AES_CMAC(&meter_keys->confidentiality_key[0], &input[0], 16, &mac[0]); + AES_CMAC(safeButUnsafeVectorPtr(meter_keys->confidentiality_key), + safeButUnsafeVectorPtr(input), 16, + safeButUnsafeVectorPtr(mac)); string s = bin2hex(mac); debug("(wmbus) ephemereal Kenc %s\n", s.c_str()); tpl_generated_key.clear(); @@ -1375,7 +1377,9 @@ bool Telegram::parseTPLConfig(std::vector::iterator &pos) mac.clear(); mac.resize(16); debugPayload("(wmbus) input to kdf for mac", input); - AES_CMAC(&meter_keys->confidentiality_key[0], &input[0], 16, &mac[0]); + AES_CMAC(safeButUnsafeVectorPtr(meter_keys->confidentiality_key), + safeButUnsafeVectorPtr(input), 16, + safeButUnsafeVectorPtr(mac)); s = bin2hex(mac); debug("(wmbus) ephemereal Kmac %s\n", s.c_str()); tpl_generated_mac_key.clear(); @@ -1474,7 +1478,9 @@ bool Telegram::checkMAC(std::vector &frame, input.insert(input.end(), from, to); string s = bin2hex(input); debug("(wmbus) input to mac %s\n", s.c_str()); - AES_CMAC(&mackey[0], &input[0], input.size(), &mac[0]); + AES_CMAC(safeButUnsafeVectorPtr(mackey), + safeButUnsafeVectorPtr(input), input.size(), + safeButUnsafeVectorPtr(mac)); string calculated = bin2hex(mac); debug("(wmbus) calculated mac %s\n", calculated.c_str()); string received = bin2hex(inmac); @@ -2351,7 +2357,7 @@ string ccType(int cc_field) if (cc_field & CC_R_RELAYED_BIT) s+= "relayed "; // Relayed by a repeater if (cc_field & CC_P_HIGH_PRIO_BIT) s+= "prio "; - if (s.back() == ' ') s.pop_back(); + if (s.size() > 0 && s.back() == ' ') s.pop_back(); return s; } @@ -4483,7 +4489,7 @@ bool trimCRCsFrameFormatAInternal(std::vector &payload, bool fail_is_ok) vector out; - uint16_t calc_crc = crc16_EN13757(&payload[0], 10); + uint16_t calc_crc = crc16_EN13757(safeButUnsafeVectorPtr(payload), 10); uint16_t check_crc = payload[10] << 8 | payload[11]; if (calc_crc != check_crc && !FUZZING) @@ -4586,7 +4592,7 @@ bool trimCRCsFrameFormatBInternal(std::vector &payload, bool fail_is_ok) crc2_pos = len-2; } - uint16_t calc_crc = crc16_EN13757(&payload[0], crc1_pos); + uint16_t calc_crc = crc16_EN13757(safeButUnsafeVectorPtr(payload), crc1_pos); uint16_t check_crc = payload[crc1_pos] << 8 | payload[crc1_pos+1]; if (calc_crc != check_crc && !FUZZING) @@ -4859,7 +4865,7 @@ string decodeTPLStatusByteOnlyStandardBits(uchar sts) if ((sts & 0x08) == 0x08) s += "PERMANENT_ERROR "; // E.g. meter needs service to work again. if ((sts & 0x10) == 0x10) s += "TEMPORARY_ERROR "; - while (s.back() == ' ') s.pop_back(); + while (s.size() > 0 && s.back() == ' ') s.pop_back(); return s; } @@ -4884,7 +4890,7 @@ string decodeTPLStatusByteWithLookup(uchar sts, map *vendor_lookup) // We could not translate, just print the bits. t += tostrprintf("UNKNOWN_%02X ", sts & 0xe0); } - while (t.back() == ' ') t.pop_back(); + while (t.size() > 0 && t.back() == ' ') t.pop_back(); } if (t == "OK") return s; diff --git a/src/wmbus_utils.cc b/src/wmbus_utils.cc index e503f18..99ddc1f 100644 --- a/src/wmbus_utils.cc +++ b/src/wmbus_utils.cc @@ -64,7 +64,7 @@ bool decrypt_ELL_AES_CTR(Telegram *t, vector &frame, vector::itera // Generate the pseudo-random bits from the IV and the key. uchar xordata[16]; - AES_ECB_encrypt(iv, &aeskey[0], xordata, 16); + AES_ECB_encrypt(iv, safeButUnsafeVectorPtr(aeskey), xordata, 16); // Xor the data with the pseudo-random bits to decrypt into tmp. uchar tmp[block_size]; @@ -170,10 +170,10 @@ bool decrypt_TPL_AES_CBC_IV(Telegram *t, debug("(TPL) IV %s\n", s.c_str()); uchar buffer_data[num_bytes_to_decrypt]; - memcpy(buffer_data, &buffer[0], num_bytes_to_decrypt); + memcpy(buffer_data, safeButUnsafeVectorPtr(buffer), num_bytes_to_decrypt); uchar decrypted_data[num_bytes_to_decrypt]; - AES_CBC_decrypt_buffer(decrypted_data, buffer_data, num_bytes_to_decrypt, &aeskey[0], iv); + AES_CBC_decrypt_buffer(decrypted_data, buffer_data, num_bytes_to_decrypt, safeButUnsafeVectorPtr(aeskey), iv); // Remove the encrypted bytes. frame.erase(pos, frame.end()); @@ -242,10 +242,10 @@ bool decrypt_TPL_AES_CBC_NO_IV(Telegram *t, vector &frame, vector: debug("(TPL) IV %s\n", s.c_str()); uchar buffer_data[num_bytes_to_decrypt]; - memcpy(buffer_data, &buffer[0], num_bytes_to_decrypt); + memcpy(buffer_data, safeButUnsafeVectorPtr(buffer), num_bytes_to_decrypt); uchar decrypted_data[num_bytes_to_decrypt]; - AES_CBC_decrypt_buffer(decrypted_data, buffer_data, num_bytes_to_decrypt, &aeskey[0], iv); + AES_CBC_decrypt_buffer(decrypted_data, buffer_data, num_bytes_to_decrypt, safeButUnsafeVectorPtr(aeskey), iv); // Remove the encrypted bytes and any potentially not decryptes bytes after. frame.erase(pos, frame.end());