Skip to content

Commit 2baa279

Browse files
wferichrissie-c
authored andcommitted
Let remote_tempdir() assume a NUL-terminated name
This is the case already. We also fix a buffer overflow opportunity in the memcpy() call by this change. Conflicts: lib/ipc_shm.c
1 parent e26ad0d commit 2baa279

File tree

5 files changed

+10
-12
lines changed

5 files changed

+10
-12
lines changed

lib/ipc_int.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,6 @@ int32_t qb_ipc_us_sock_error_is_disconnected(int err);
208208

209209
int use_filesystem_sockets(void);
210210

211-
void remove_tempdir(const char *name, size_t namelen);
211+
void remove_tempdir(const char *name);
212212

213213
#endif /* QB_IPC_INT_H_DEFINED */

lib/ipc_setup.c

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -914,16 +914,15 @@ qb_ipcs_us_connection_acceptor(int fd, int revent, void *data)
914914
return 0;
915915
}
916916

917-
void remove_tempdir(const char *name, size_t namelen)
917+
void remove_tempdir(const char *name)
918918
{
919919
#if defined(QB_LINUX) || defined(QB_CYGWIN)
920920
char dirname[PATH_MAX];
921-
char *slash;
922-
memcpy(dirname, name, namelen);
921+
char *slash = strrchr(name, '/');
923922

924-
slash = strrchr(dirname, '/');
925-
if (slash) {
926-
*slash = '\0';
923+
if (slash && slash - name < sizeof dirname) {
924+
memcpy(dirname, name, slash - name);
925+
dirname[slash - name] = '\0';
927926
/* This gets called more than it needs to be really, so we don't check
928927
* the return code. It's more of a desperate attempt to clean up after ourself
929928
* in either the server or client.

lib/ipc_shm.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -266,10 +266,9 @@ qb_ipcs_shm_disconnect(struct qb_ipcs_connection *c)
266266
}
267267
}
268268

269-
remove_tempdir(c->description, CONNECTION_DESCRIPTION);
270-
271269
end_disconnect:
272270
sigaction(SIGBUS, &old_sa, NULL);
271+
remove_tempdir(c->description);
273272
}
274273

275274
static int32_t

lib/ipc_socket.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -376,7 +376,7 @@ qb_ipcc_us_disconnect(struct qb_ipcc_connection *c)
376376
}
377377

378378
/* Last-ditch attempt to tidy up after ourself */
379-
remove_tempdir(c->request.u.us.shared_file_name, PATH_MAX);
379+
remove_tempdir(c->request.u.us.shared_file_name);
380380

381381
qb_ipcc_us_sock_close(c->event.u.us.sock);
382382
qb_ipcc_us_sock_close(c->request.u.us.sock);
@@ -772,7 +772,7 @@ qb_ipcs_us_disconnect(struct qb_ipcs_connection *c)
772772

773773

774774
}
775-
remove_tempdir(c->description, CONNECTION_DESCRIPTION);
775+
remove_tempdir(c->description);
776776
}
777777

778778
static int32_t

lib/ipcs.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -642,7 +642,7 @@ qb_ipcs_disconnect(struct qb_ipcs_connection *c)
642642
scheduled_retry = 1;
643643
}
644644
}
645-
remove_tempdir(c->description, CONNECTION_DESCRIPTION);
645+
remove_tempdir(c->description);
646646
if (scheduled_retry == 0) {
647647
/* This removes the initial alloc ref */
648648
qb_ipcs_connection_unref(c);

0 commit comments

Comments
 (0)