Skip to content

Commit c1812e0

Browse files
committed
net_msp: Avoid potential use after free with config values.
This is unlikely, but we were keeping references to config values after the config was unlocked; despite not freeing it, the values could potentially become invalid in the future. Do everything we need to do with these values before unlocking the config, to avoid any potential issues.
1 parent 80d59c2 commit c1812e0

1 file changed

Lines changed: 22 additions & 25 deletions

File tree

nets/net_msp.c

Lines changed: 22 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -399,50 +399,47 @@ static void *msp_udp_listener(void *varg)
399399
return NULL;
400400
}
401401

402-
static const char *ip = NULL, *interface = NULL;
403-
404402
static int load_config(void)
405403
{
406404
int res;
405+
const char *ip = NULL, *interface = NULL;
407406
struct bbs_config *cfg = bbs_config_load("net_msp.conf", 0);
408407

409408
if (!cfg) {
410409
return 0;
411410
}
412411

413-
/* We don't destroy the config after we return, so it's okay that we have a constant reference to it directly */
414412
ip = bbs_config_val(cfg, "udp", "ip");
415413
interface = bbs_config_val(cfg, "udp", "interface");
416414

415+
/* At least one of the ports must be enabled: */
417416
res = bbs_config_val_set_port(cfg, "ports", "tcp", &msp_tcp_port) && bbs_config_val_set_port(cfg, "ports", "udp", &msp_udp_port);
417+
if (!res) {
418+
/* Use ip and interface before returning; even though we don't destroy the config after we return,
419+
* they could nonetheless become invalid references after config is unlocked */
420+
if (msp_tcp_port) {
421+
res = bbs_start_tcp_listener(msp_tcp_port, "MSP", msp_tcp_handler);
422+
}
423+
if (!res && msp_udp_port) {
424+
/* Be extra careful about the interfaces to which we bind,
425+
* since the source IP of UDP messages can be spoofed,
426+
* and we won't be able to tell. */
427+
res = bbs_make_udp_socket(&udp_socket, msp_udp_port, ip, interface);
428+
if (!res) {
429+
res = bbs_pthread_create(&udp_thread, NULL, msp_udp_listener, NULL);
430+
}
431+
if (res) {
432+
bbs_stop_tcp_listener(msp_tcp_port);
433+
}
434+
}
435+
}
418436
bbs_config_unlock(cfg);
419437
return res;
420438
}
421439

422440
static int load_module(void)
423441
{
424-
int res = 0;
425-
426-
if (load_config()) {
427-
return -1;
428-
}
429-
430-
if (msp_tcp_port) {
431-
res = bbs_start_tcp_listener(msp_tcp_port, "MSP", msp_tcp_handler);
432-
}
433-
if (!res && msp_udp_port) {
434-
/* Be extra careful about the interfaces to which we bind,
435-
* since the source IP of UDP messages can be spoofed,
436-
* and we won't be able to tell. */
437-
res = bbs_make_udp_socket(&udp_socket, msp_udp_port, ip, interface);
438-
if (!res) {
439-
res = bbs_pthread_create(&udp_thread, NULL, msp_udp_listener, NULL);
440-
}
441-
if (res) {
442-
bbs_stop_tcp_listener(msp_tcp_port);
443-
}
444-
}
445-
return res;
442+
return load_config() ? -1 : 0;
446443
}
447444

448445
static int unload_module(void)

0 commit comments

Comments
 (0)