Hi,
Please find the latest report on new defect(s) introduced to Synchronet found with Coverity Scan.
15 new defect(s) introduced to Synchronet found with Coverity Scan.
1 defect(s), reported by Coverity Scan earlier, were marked fixed in the recent build analyzed by Coverity Scan.
New defect(s) Reported-by: Coverity Scan
Showing 15 of 15 defect(s)
** CID 646018: Error handling issues (CHECKED_RETURN)
/mqtt_broker.cpp: 561 in mqtt5::Broker::broker_thread()()
_____________________________________________________________________________________________
*** CID 646018: Error handling issues (CHECKED_RETURN) /mqtt_broker.cpp: 561 in mqtt5::Broker::broker_thread()()
555 break;
556 }
557
558 #ifndef _WIN32
559 if (m_wakeup_pipe[0] >= 0 && FD_ISSET(m_wakeup_pipe[0], &rfds)) {
560 char buf[64];
CID 646018: Error handling issues (CHECKED_RETURN)
"read(int, void *, size_t)" returns the number of bytes read, but it is ignored.
561 (void)read(m_wakeup_pipe[0], buf, sizeof(buf)); 562 }
563 #endif
564
565 if (m_listen_sock >= 0 && FD_ISSET(m_listen_sock, &rfds))
566 accept_connection(m_listen_sock);
** CID 646017: (CHECKED_RETURN)
/mqtt_broker.cpp: 175 in mqtt5::Broker::start(scfg_t *, unsigned short, int (*)(void *, int, const char *), void *)()
/mqtt_broker.cpp: 181 in mqtt5::Broker::start(scfg_t *, unsigned short, int (*)(void *, int, const char *), void *)()
/mqtt_broker.cpp: 166 in mqtt5::Broker::start(scfg_t *, unsigned short, int (*)(void *, int, const char *), void *)()
_____________________________________________________________________________________________
*** CID 646017: (CHECKED_RETURN)
/mqtt_broker.cpp: 175 in mqtt5::Broker::start(scfg_t *, unsigned short, int (*)(void *, int, const char *), void *)()
169 closesocket(m_listen_sock);
170 m_listen_sock = -1;
171 return false;
172 }
173 } else {
174 int off = 0;
CID 646017: (CHECKED_RETURN)
Calling "setsockopt(this->m_listen_sock, IPPROTO_IPV6, 26, (char const *)&off, 4U)" without checking return value. This library function may fail and return an error code.
175 setsockopt(m_listen_sock, IPPROTO_IPV6, IPV6_V6ONLY, (const char *)&off, sizeof(off));
176 struct sockaddr_in6 addr = {};
177 addr.sin6_family = AF_INET6;
178 addr.sin6_port = htons(port);
179 addr.sin6_addr = in6addr_any;
180 int opt = 1;
/mqtt_broker.cpp: 181 in mqtt5::Broker::start(scfg_t *, unsigned short, int (*)(void *, int, const char *), void *)()
175 setsockopt(m_listen_sock, IPPROTO_IPV6, IPV6_V6ONLY, (const char *)&off, sizeof(off));
176 struct sockaddr_in6 addr = {};
177 addr.sin6_family = AF_INET6;
178 addr.sin6_port = htons(port);
179 addr.sin6_addr = in6addr_any;
180 int opt = 1;
CID 646017: (CHECKED_RETURN)
Calling "setsockopt(this->m_listen_sock, 1, 2, (char const *)&opt, 4U)" without checking return value. This library function may fail and return an error code.
181 setsockopt(m_listen_sock, SOL_SOCKET, SO_REUSEADDR, (const char *)&opt, sizeof(opt));
182 if (bind(m_listen_sock, (struct sockaddr *)&addr, sizeof(addr)) < 0) {
183 log(LOG_ERR, "MQTT broker: bind() port %u failed: %d", port, ERROR_VALUE);
184 closesocket(m_listen_sock);
185 m_listen_sock = -1;
186 return false;
/mqtt_broker.cpp: 166 in mqtt5::Broker::start(scfg_t *, unsigned short, int (*)(void *, int, const char *), void *)()
160 }
161 struct sockaddr_in addr = {};
162 addr.sin_family = AF_INET;
163 addr.sin_port = htons(port);
164 addr.sin_addr.s_addr = INADDR_ANY;
165 int opt = 1;
CID 646017: (CHECKED_RETURN)
Calling "setsockopt(this->m_listen_sock, 1, 2, (char const *)&opt, 4U)" without checking return value. This library function may fail and return an error code.
166 setsockopt(m_listen_sock, SOL_SOCKET, SO_REUSEADDR, (const char *)&opt, sizeof(opt));
167 if (bind(m_listen_sock, (struct sockaddr *)&addr, sizeof(addr)) < 0) {
168 log(LOG_ERR, "MQTT broker: bind() port %u failed: %d", port, ERROR_VALUE);
169 closesocket(m_listen_sock);
170 m_listen_sock = -1;
171 return false;
** CID 646016: Performance inefficiencies (COPY_INSTEAD_OF_MOVE) /mqtt_broker.cpp: 347 in mqtt5::Broker::local_subscribe(mqtt5::LocalClient *, const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> &, int)::[lambda(const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> &, std::shared_ptr<mqtt5::Message>) (instance 1)]::operator ()(const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> &, std::shared_ptr<mqtt5::Message>) const()
_____________________________________________________________________________________________
*** CID 646016: Performance inefficiencies (COPY_INSTEAD_OF_MOVE) /mqtt_broker.cpp: 347 in mqtt5::Broker::local_subscribe(mqtt5::LocalClient *, const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> &, int)::[lambda(const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> &, std::shared_ptr<mqtt5::Message>) (instance 1)]::operator ()(const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> &, std::shared_ptr<mqtt5::Message>) const()
341 {
342 std::lock_guard<std::mutex> slock(client->sub_mutex); 343 client->subscriptions.push_back(topic);
344 }
345
346 m_topics.match_retained(topic, [&](const std::string &, std::shared_ptr<Message> msg) {
CID 646016: Performance inefficiencies (COPY_INSTEAD_OF_MOVE) >>> "msg" is passed-by-value as parameter to "std::shared_ptr<mqtt5::Message>::shared_ptr(std::shared_ptr<mqtt5::Message> const &) /*explicit =default*/", when it could be moved instead.
347 if (msg) deliver_to_local(client, msg);
348 });
349
350 return 0;
351 }
352
** CID 646015: Error handling issues (CHECKED_RETURN)
/mqtt_broker.cpp: 636 in mqtt5::Broker::accept_connection(int)()
_____________________________________________________________________________________________
*** CID 646015: Error handling issues (CHECKED_RETURN) /mqtt_broker.cpp: 636 in mqtt5::Broker::accept_connection(int)() 630 struct sockaddr_storage addr;
631 socklen_t addrlen = sizeof(addr);
632 int sock = accept(listen_sock, (struct sockaddr *)&addr, &addrlen);
633 if (sock < 0) return;
634
635 int nodelay = 1;
CID 646015: Error handling issues (CHECKED_RETURN)
Calling "setsockopt(sock, IPPROTO_TCP, 1, (char *)&nodelay, 4U)" without checking return value. This library function may fail and return an error code.
636 setsockopt(sock, IPPROTO_TCP, TCP_NODELAY, (char *)&nodelay, sizeof(nodelay));
637
638 CRYPT_SESSION tls_sess = CRYPT_UNUSED;
639 int ret;
640
641 if (!do_cryptInit(broker_lprintf)) {
** CID 646029: Insecure data handling (TAINTED_SCALAR) /mqtt_protocol.cpp: 174 in mqtt5::Reader::read_utf8(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> &)()
_____________________________________________________________________________________________
*** CID 646029: Insecure data handling (TAINTED_SCALAR) /mqtt_protocol.cpp: 174 in mqtt5::Reader::read_utf8(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> &)()
168
169 bool Reader::read_utf8(std::string &out)
170 {
171 uint16_t len;
172 if (!read_u16(len)) return false;
173 if (m_pos + len > m_len) return false;
CID 646029: Insecure data handling (TAINTED_SCALAR)
Passing tainted expression "len" to "assign", which uses it as an offset. [Note: The source code implementation of the function has been overridden by a builtin model.]
174 out.assign(reinterpret_cast<const char *>(m_data + m_pos), len);
175 m_pos += len;
176 return true;
177 }
178
179 bool Reader::read_binary(std::vector<uint8_t> &out)
** CID 646028: Performance inefficiencies (AUTO_CAUSES_COPY) /mqtt_broker.cpp: 365 in mqtt5::Broker::route_publish(const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> &, std::shared_ptr<mqtt5::Message>)::[lambda(const mqtt5::Subscriber &) (instance 1)]::operator ()(const mqtt5::Subscriber &) const()
_____________________________________________________________________________________________
*** CID 646028: Performance inefficiencies (AUTO_CAUSES_COPY) /mqtt_broker.cpp: 365 in mqtt5::Broker::route_publish(const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> &, std::shared_ptr<mqtt5::Message>)::[lambda(const mqtt5::Subscriber &) (instance 1)]::operator ()(const mqtt5::Subscriber &) const()
359
360 std::unordered_map<std::string, const SubscriptionOptions *> delivered;
361
362 m_topics.publish(msg->topic, [&](const Subscriber &sub) {
363 if (sub.opts.no_local && sub.client_id == sender_id) 364 return;
CID 646028: Performance inefficiencies (AUTO_CAUSES_COPY)
Using the "auto" keyword without an "&" causes the copy of an object of type "std::string".
365 auto key = sub.client_id;
366 if (delivered.count(key))
367 return;
368 delivered[key] = &sub.opts;
369
370 for (const auto &lc : m_local_clients) {
** CID 646027: Performance inefficiencies (COPY_INSTEAD_OF_MOVE) /mqtt_broker.cpp: 303 in mqtt5::Broker::publish_sys(const char *, const void *, unsigned long)()
_____________________________________________________________________________________________
*** CID 646027: Performance inefficiencies (COPY_INSTEAD_OF_MOVE) /mqtt_broker.cpp: 303 in mqtt5::Broker::publish_sys(const char *, const void *, unsigned long)()
297 if (payload && len > 0)
298 msg->payload.assign(static_cast<const uint8_t *>(payload),
299 static_cast<const uint8_t *>(payload) + len);
300 msg->created_at = time(nullptr);
301
302 std::lock_guard<std::recursive_mutex> lock(m_mutex);
CID 646027: Performance inefficiencies (COPY_INSTEAD_OF_MOVE) >>> "msg" is passed-by-value as parameter to "std::shared_ptr<mqtt5::Message>::shared_ptr(std::shared_ptr<mqtt5::Message> const &) /*explicit =default*/", when it could be moved instead.
303 route_publish("$SYS", msg);
304 }
305
306 void Broker::deregister_local(LocalClient *client)
307 {
308 std::lock_guard<std::recursive_mutex> lock(m_mutex);
** CID 646026: Performance inefficiencies (COPY_INSTEAD_OF_MOVE) /mqtt_broker.cpp: 101 in mqtt5::Broker::build_psk_table()()
_____________________________________________________________________________________________
*** CID 646026: Performance inefficiencies (COPY_INSTEAD_OF_MOVE) /mqtt_broker.cpp: 101 in mqtt5::Broker::build_psk_table()()
95 if (user.misc & (DELETED | INACTIVE))
96 continue;
97 std::string alias(user.alias);
98 std::string pass(user.pass);
99 std::transform(alias.begin(), alias.end(), alias.begin(), ::tolower);
100 std::transform(pass.begin(), pass.end(), pass.begin(), ::tolower);
CID 646026: Performance inefficiencies (COPY_INSTEAD_OF_MOVE) >>> "pass" is copied in call to copy assignment for class "std::string", when it could be moved instead.
101 m_psk_table[alias] = pass;
102 }
103 log(LOG_DEBUG, "MQTT broker: loaded %zu PSK identities", m_psk_table.size());
104 }
105
106 bool Broker::authenticate_psk(const std::string &identity, const std::string &password)
** CID 646025: Performance inefficiencies (COPY_INSTEAD_OF_MOVE) /mqtt_broker.cpp: 971 in mqtt5::Broker::handle_subscribe(mqtt5::NetworkSession &, const unsigned char *, unsigned long)::[lambda(const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> &, std::shared_ptr<mqtt5::Message>) (instance 1)]::operator ()(const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> &, std::shared_ptr<mqtt5::Message>) const()
_____________________________________________________________________________________________
*** CID 646025: Performance inefficiencies (COPY_INSTEAD_OF_MOVE) /mqtt_broker.cpp: 971 in mqtt5::Broker::handle_subscribe(mqtt5::NetworkSession &, const unsigned char *, unsigned long)::[lambda(const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> &, std::shared_ptr<mqtt5::Message>) (instance 1)]::operator ()(const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> &, std::shared_ptr<mqtt5::Message>) const()
965 if (!msg) return;
966 if (msg->props.has(PROP_MESSAGE_EXPIRY)) {
967 uint32_t expiry = msg->props.get_u32(PROP_MESSAGE_EXPIRY, 0);
968 if (time(nullptr) - msg->created_at > (time_t)expiry)
969 return;
970 }
CID 646025: Performance inefficiencies (COPY_INSTEAD_OF_MOVE) >>> "msg" is passed-by-value as parameter to "std::shared_ptr<mqtt5::Message>::shared_ptr(std::shared_ptr<mqtt5::Message> const &) /*explicit =default*/", when it could be moved instead.
971 deliver_to_network(session, msg, opts); 972 });
973 }
974
975 reason_codes.push_back(tf.qos());
976 }
** CID 646024: Performance inefficiencies (COPY_INSTEAD_OF_MOVE) /mqtt_broker.cpp: 224 in mqtt5::Broker::start(scfg_t *, unsigned short, int (*)(void *, int, const char *), void *)()
_____________________________________________________________________________________________
*** CID 646024: Performance inefficiencies (COPY_INSTEAD_OF_MOVE) /mqtt_broker.cpp: 224 in mqtt5::Broker::start(scfg_t *, unsigned short, int (*)(void *, int, const char *), void *)()
218 auto msg = std::make_shared<Message>();
219 msg->type = PUBLISH;
220 msg->flags = 1;
221 msg->topic = "$SYS/broker/version";
222 msg->payload.assign(ver, ver + strlen(ver));
223 msg->created_at = time(nullptr);
CID 646024: Performance inefficiencies (COPY_INSTEAD_OF_MOVE) >>> "msg" is passed-by-value as parameter to "std::shared_ptr<mqtt5::Message>::shared_ptr(std::shared_ptr<mqtt5::Message> const &) /*explicit =default*/", when it could be moved instead.
224 m_topics.set_retained("$SYS/broker/version", msg);
225 }
226
227 log(LOG_INFO, "MQTT broker: listening on port %u", port);
228 return true;
229 }
** CID 646023: (CHECKED_RETURN)
/mqtt_broker.cpp: 204 in mqtt5::Broker::start(scfg_t *, unsigned short, int (*)(void *, int, const char *), void *)()
/mqtt_broker.cpp: 205 in mqtt5::Broker::start(scfg_t *, unsigned short, int (*)(void *, int, const char *), void *)()
_____________________________________________________________________________________________
*** CID 646023: (CHECKED_RETURN)
/mqtt_broker.cpp: 204 in mqtt5::Broker::start(scfg_t *, unsigned short, int (*)(void *, int, const char *), void *)()
198 if (pipe(m_wakeup_pipe) < 0) {
199 log(LOG_ERR, "MQTT broker: pipe() failed: %d", errno); 200 closesocket(m_listen_sock);
201 m_listen_sock = -1;
202 return false;
203 }
CID 646023: (CHECKED_RETURN)
Calling "fcntl(this->m_wakeup_pipe[0], 4, 2048)" without checking return value. This library function may fail and return an error code.
204 fcntl(m_wakeup_pipe[0], F_SETFL, O_NONBLOCK);
205 fcntl(m_wakeup_pipe[1], F_SETFL, O_NONBLOCK);
206 #endif
207
208 m_running = true;
209 m_thread = std::thread(&Broker::broker_thread, this); /mqtt_broker.cpp: 205 in mqtt5::Broker::start(scfg_t *, unsigned short, int (*)(void *, int, const char *), void *)()
199 log(LOG_ERR, "MQTT broker: pipe() failed: %d", errno); 200 closesocket(m_listen_sock);
201 m_listen_sock = -1;
202 return false;
203 }
204 fcntl(m_wakeup_pipe[0], F_SETFL, O_NONBLOCK);
CID 646023: (CHECKED_RETURN)
Calling "fcntl(this->m_wakeup_pipe[1], 4, 2048)" without checking return value. This library function may fail and return an error code.
205 fcntl(m_wakeup_pipe[1], F_SETFL, O_NONBLOCK);
206 #endif
207
208 m_running = true;
209 m_thread = std::thread(&Broker::broker_thread, this);
210
** CID 646022: Performance inefficiencies (COPY_INSTEAD_OF_MOVE) /mqtt_broker.cpp: 715 in mqtt5::Broker::accept_connection(int)()
_____________________________________________________________________________________________
*** CID 646022: Performance inefficiencies (COPY_INSTEAD_OF_MOVE) /mqtt_broker.cpp: 715 in mqtt5::Broker::accept_connection(int)() 709
710 std::string temp_id = "pending-" + std::to_string(sock);
711 std::lock_guard<std::recursive_mutex> lock(m_mutex);
712 auto &session = m_sessions[temp_id];
713 session.socket = sock;
714 session.tls_sess = tls_sess;
CID 646022: Performance inefficiencies (COPY_INSTEAD_OF_MOVE) >>> "psk_id" is copied in call to copy assignment for class "std::string", when it could be moved instead.
715 session.tls_psk_id = psk_id;
716 session.last_activity = time(nullptr);
717 log(LOG_DEBUG, "MQTT broker: new TLS connection on socket %d", sock);
718 }
719
720 // Network data handling
** CID 646021: Concurrent data access violations (MISSING_LOCK) /mqtt_broker.cpp: 57 in mqtt5::Broker::instance()()
_____________________________________________________________________________________________
*** CID 646021: Concurrent data access violations (MISSING_LOCK) /mqtt_broker.cpp: 57 in mqtt5::Broker::instance()()
51
52 Broker *Broker::s_instance = nullptr;
53 std::mutex Broker::s_instance_mutex;
54
55 Broker *Broker::instance()
56 {
CID 646021: Concurrent data access violations (MISSING_LOCK) >>> Accessing "mqtt5::Broker::s_instance" without holding lock "mqtt5::Broker::s_instance_mutex". Elsewhere, "mqtt5::Broker::s_instance" is written to with "mqtt5::Broker::s_instance_mutex" held 2 out of 2 times (1 of these accesses strongly imply that it is necessary).
57 return s_instance;
58 }
59
60 Broker::Broker()
61 {
62 }
** CID 646020: Performance inefficiencies (COPY_INSTEAD_OF_MOVE) /mqtt_broker.cpp: 915 in mqtt5::Broker::handle_publish(mqtt5::NetworkSession &, unsigned char, const unsigned char *, unsigned long)()
_____________________________________________________________________________________________
*** CID 646020: Performance inefficiencies (COPY_INSTEAD_OF_MOVE) /mqtt_broker.cpp: 915 in mqtt5::Broker::handle_publish(mqtt5::NetworkSession &, unsigned char, const unsigned char *, unsigned long)()
909 teardown_network(session, 0x81);
910 return;
911 }
912
913 uint8_t qos = msg->qos();
914
CID 646020: Performance inefficiencies (COPY_INSTEAD_OF_MOVE) >>> "msg" is passed-by-value as parameter to "std::shared_ptr<mqtt5::Message>::shared_ptr(std::shared_ptr<mqtt5::Message> const &) /*explicit =default*/", when it could be moved instead.
915 route_publish(session.client_id, msg);
916
917 if (qos == 1) {
918 send_to_network(session, build_ack(PUBACK, pid, 0)); 919 } else if (qos == 2) {
920 session.rx_unacked[pid] = true;
** CID 646019: Performance inefficiencies (COPY_INSTEAD_OF_MOVE) /mqtt_broker.cpp: 413 in mqtt5::Broker::deliver_to_network(mqtt5::NetworkSession &, std::shared_ptr<mqtt5::Message>, const mqtt5::SubscriptionOptions &)()
_____________________________________________________________________________________________
*** CID 646019: Performance inefficiencies (COPY_INSTEAD_OF_MOVE) /mqtt_broker.cpp: 413 in mqtt5::Broker::deliver_to_network(mqtt5::NetworkSession &, std::shared_ptr<mqtt5::Message>, const mqtt5::SubscriptionOptions &)()
407 auto pkt = build_publish(*msg, pid, false, qos,
408 extra.empty() ? nullptr : &extra); 409 send_to_network(session, pkt);
410
411 if (qos > 0) {
412 Queued q;
CID 646019: Performance inefficiencies (COPY_INSTEAD_OF_MOVE) >>> "msg" is copied in call to copy assignment for class "std::shared_ptr<mqtt5::Message>", when it could be moved instead.
413 q.msg = msg;
414 q.pid = pid;
415 session.tx_unacked[pid] = std::move(q);
416 }
417 }
418
________________________________________________________________________________________________________
To view the defects in Coverity Scan visit,
https://scan.coverity.com/projects/synchronet?tab=overview
---
* Synchronet * Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net