Skip to content

Commit

Permalink
Eradicate VLAs from the codebase
Browse files Browse the repository at this point in the history
They are already banned in e.g. the Linux kernel, and are considered a
security risk by many projects.

Also fix a -Wformat-nonliteral warning from clang.
  • Loading branch information
DemiMarie committed Apr 28, 2024
1 parent 82cbe71 commit 56fdd7d
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 33 deletions.
2 changes: 1 addition & 1 deletion daemon/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ override QUBES_CFLAGS:=-I../libqrexec -g -O2 -Wall -Wextra -Werror -fPIC \
-D_FORTIFY_SOURCE=2 -fstack-protector-strong -std=gnu11 -D_POSIX_C_SOURCE=200809L \
-D_GNU_SOURCE $(CFLAGS) \
-Wstrict-prototypes -Wold-style-definition -Wmissing-declarations \
-fvisibility=hidden -fno-strict-aliasing
-fvisibility=hidden -fno-strict-aliasing -Wvla
override LDFLAGS += -pie -Wl,-z,relro,-z,now -L../libqrexec
override LDLIBS += $(shell pkg-config --libs $(VCHAN_PKG)) -lqrexec-utils

Expand Down
45 changes: 19 additions & 26 deletions daemon/qrexec-daemon.c
Original file line number Diff line number Diff line change
Expand Up @@ -140,23 +140,19 @@ static int remote_domain_id;

static void unlink_qrexec_socket(void)
{
char socket_address[40];
char link_to_socket_name[strlen(remote_domain_name) + sizeof(socket_address)];
char *socket_address;
char *link_to_socket_name;

int v = snprintf(socket_address, sizeof(socket_address),
"qrexec.%d", remote_domain_id);
if (v < (int)sizeof("qrexec.1") - 1 || v >= (int)sizeof(socket_address))
abort();
v = snprintf(link_to_socket_name, sizeof(link_to_socket_name),
"qrexec.%s", remote_domain_name);
if (v < (int)sizeof("qrexec.") - 1 || v >= (int)sizeof(link_to_socket_name))
abort();
v = unlink(socket_address);
if (v != 0 && !(v == -1 && errno == ENOENT))
if (asprintf(&socket_address, "%s/qrexec.%d", socket_dir, remote_domain_id) < 0)
err(1, "asprintf");
if (unlink(socket_address) != 0 && errno != ENOENT)
err(1, "unlink(%s)", socket_address);
v = unlink(link_to_socket_name);
if (v != 0 && !(v == -1 && errno == ENOENT))
free(socket_address);
if (asprintf(&link_to_socket_name, "%s/qrexec.%s", socket_dir, remote_domain_name) < 0)
err(1, "asprintf");
if (unlink(link_to_socket_name) != 0 && errno != ENOENT)
err(1, "unlink(%s)", link_to_socket_name);
free(link_to_socket_name);
}

static void handle_vchan_error(const char *op)
Expand All @@ -169,18 +165,14 @@ static void handle_vchan_error(const char *op)
static int create_qrexec_socket(int domid, const char *domname)
{
char socket_address[40];
char link_to_socket_name[strlen(domname) + sizeof(socket_address)];
int res;

if ((unsigned)snprintf(socket_address, sizeof(socket_address),
"qrexec.%d", domid) >= sizeof(socket_address))
errx(1, "socket name too long");
if ((unsigned)snprintf(link_to_socket_name, sizeof link_to_socket_name,
"qrexec.%s", domname) >= sizeof link_to_socket_name)
errx(1, "socket link name too long");
res = unlink(link_to_socket_name);
if (res != 0 && !(res == -1 && errno == ENOENT))
err(1, "unlink(%s)", link_to_socket_name);
char *link_to_socket_name;

snprintf(socket_address, sizeof(socket_address),
"%s/qrexec.%d", socket_dir, domid);
if (asprintf(&link_to_socket_name,
"%s/qrexec.%s", socket_dir, domname) < 0)
err(1, "asprintf");
unlink(link_to_socket_name);

/* When running as root, make the socket accessible; perms on /var/run/qubes still apply */
umask(0);
Expand All @@ -189,6 +181,7 @@ static int create_qrexec_socket(int domid, const char *domname)
}
int fd = get_server_socket(socket_address);
umask(0077);
free(link_to_socket_name);
return fd;
}

Expand Down
3 changes: 2 additions & 1 deletion libqrexec/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ override QUBES_CFLAGS := -I. -I../libqrexec -g -O2 -Wall -Wextra -Werror \
-D_FORTIFY_SOURCE=2 -fstack-protector-strong -fPIC -std=gnu11 -D_POSIX_C_SOURCE=200809L \
-D_GNU_SOURCE $(CFLAGS) \
-Wstrict-prototypes -Wold-style-definition -Wmissing-declarations \
-fno-delete-null-pointer-checks -fvisibility=hidden
-fno-delete-null-pointer-checks -fvisibility=hidden \
-Wvla -Wformat=2

override LDFLAGS += -pie -Wl,-z,relro,-z,now -shared

Expand Down
13 changes: 8 additions & 5 deletions libqrexec/log.c
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
static const char *qrexec_program_name = "qrexec";

static void log_time(void) {
const size_t buf_len = 32;
#define buf_len 32
char buf[buf_len];
struct tm tm_buf;
struct tm *tm;
Expand All @@ -46,12 +46,14 @@ static void log_time(void) {

strftime(buf, buf_len, "%Y-%m-%d %H:%M:%S", tm);
fprintf(stderr, "%s.%03d ", buf, (int)(tv.tv_usec / 1000));
#undef buf_len
}

static void qrexec_logv(__attribute__((unused)) int level, int errnoval,
const char *file, int line,
const char *func, const char *fmt, va_list ap) {
const size_t buf_len = 64;
static void __attribute__((format(printf, 6, 0)))
qrexec_logv(__attribute__((unused)) int level, int errnoval,
const char *file, int line,
const char *func, const char *fmt, va_list ap) {
#define buf_len 64
char buf[buf_len];
char *err;
int _errno = errno;
Expand All @@ -65,6 +67,7 @@ static void qrexec_logv(__attribute__((unused)) int level, int errnoval,
fprintf(stderr, "\n");
fflush(stderr);
errno = _errno;
#undef buf_len
}

void qrexec_log(int level, int errnoval, const char *file, int line,
Expand Down

0 comments on commit 56fdd7d

Please sign in to comment.