Skip to content

Commit 56fdd7d

Browse files
committed
Eradicate VLAs from the codebase
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.
1 parent 82cbe71 commit 56fdd7d

File tree

4 files changed

+30
-33
lines changed

4 files changed

+30
-33
lines changed

daemon/Makefile

+1-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ override QUBES_CFLAGS:=-I../libqrexec -g -O2 -Wall -Wextra -Werror -fPIC \
55
-D_FORTIFY_SOURCE=2 -fstack-protector-strong -std=gnu11 -D_POSIX_C_SOURCE=200809L \
66
-D_GNU_SOURCE $(CFLAGS) \
77
-Wstrict-prototypes -Wold-style-definition -Wmissing-declarations \
8-
-fvisibility=hidden -fno-strict-aliasing
8+
-fvisibility=hidden -fno-strict-aliasing -Wvla
99
override LDFLAGS += -pie -Wl,-z,relro,-z,now -L../libqrexec
1010
override LDLIBS += $(shell pkg-config --libs $(VCHAN_PKG)) -lqrexec-utils
1111

daemon/qrexec-daemon.c

+19-26
Original file line numberDiff line numberDiff line change
@@ -140,23 +140,19 @@ static int remote_domain_id;
140140

141141
static void unlink_qrexec_socket(void)
142142
{
143-
char socket_address[40];
144-
char link_to_socket_name[strlen(remote_domain_name) + sizeof(socket_address)];
143+
char *socket_address;
144+
char *link_to_socket_name;
145145

146-
int v = snprintf(socket_address, sizeof(socket_address),
147-
"qrexec.%d", remote_domain_id);
148-
if (v < (int)sizeof("qrexec.1") - 1 || v >= (int)sizeof(socket_address))
149-
abort();
150-
v = snprintf(link_to_socket_name, sizeof(link_to_socket_name),
151-
"qrexec.%s", remote_domain_name);
152-
if (v < (int)sizeof("qrexec.") - 1 || v >= (int)sizeof(link_to_socket_name))
153-
abort();
154-
v = unlink(socket_address);
155-
if (v != 0 && !(v == -1 && errno == ENOENT))
146+
if (asprintf(&socket_address, "%s/qrexec.%d", socket_dir, remote_domain_id) < 0)
147+
err(1, "asprintf");
148+
if (unlink(socket_address) != 0 && errno != ENOENT)
156149
err(1, "unlink(%s)", socket_address);
157-
v = unlink(link_to_socket_name);
158-
if (v != 0 && !(v == -1 && errno == ENOENT))
150+
free(socket_address);
151+
if (asprintf(&link_to_socket_name, "%s/qrexec.%s", socket_dir, remote_domain_name) < 0)
152+
err(1, "asprintf");
153+
if (unlink(link_to_socket_name) != 0 && errno != ENOENT)
159154
err(1, "unlink(%s)", link_to_socket_name);
155+
free(link_to_socket_name);
160156
}
161157

162158
static void handle_vchan_error(const char *op)
@@ -169,18 +165,14 @@ static void handle_vchan_error(const char *op)
169165
static int create_qrexec_socket(int domid, const char *domname)
170166
{
171167
char socket_address[40];
172-
char link_to_socket_name[strlen(domname) + sizeof(socket_address)];
173-
int res;
174-
175-
if ((unsigned)snprintf(socket_address, sizeof(socket_address),
176-
"qrexec.%d", domid) >= sizeof(socket_address))
177-
errx(1, "socket name too long");
178-
if ((unsigned)snprintf(link_to_socket_name, sizeof link_to_socket_name,
179-
"qrexec.%s", domname) >= sizeof link_to_socket_name)
180-
errx(1, "socket link name too long");
181-
res = unlink(link_to_socket_name);
182-
if (res != 0 && !(res == -1 && errno == ENOENT))
183-
err(1, "unlink(%s)", link_to_socket_name);
168+
char *link_to_socket_name;
169+
170+
snprintf(socket_address, sizeof(socket_address),
171+
"%s/qrexec.%d", socket_dir, domid);
172+
if (asprintf(&link_to_socket_name,
173+
"%s/qrexec.%s", socket_dir, domname) < 0)
174+
err(1, "asprintf");
175+
unlink(link_to_socket_name);
184176

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

libqrexec/Makefile

+2-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@ override QUBES_CFLAGS := -I. -I../libqrexec -g -O2 -Wall -Wextra -Werror \
55
-D_FORTIFY_SOURCE=2 -fstack-protector-strong -fPIC -std=gnu11 -D_POSIX_C_SOURCE=200809L \
66
-D_GNU_SOURCE $(CFLAGS) \
77
-Wstrict-prototypes -Wold-style-definition -Wmissing-declarations \
8-
-fno-delete-null-pointer-checks -fvisibility=hidden
8+
-fno-delete-null-pointer-checks -fvisibility=hidden \
9+
-Wvla -Wformat=2
910

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

libqrexec/log.c

+8-5
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232
static const char *qrexec_program_name = "qrexec";
3333

3434
static void log_time(void) {
35-
const size_t buf_len = 32;
35+
#define buf_len 32
3636
char buf[buf_len];
3737
struct tm tm_buf;
3838
struct tm *tm;
@@ -46,12 +46,14 @@ static void log_time(void) {
4646

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

51-
static void qrexec_logv(__attribute__((unused)) int level, int errnoval,
52-
const char *file, int line,
53-
const char *func, const char *fmt, va_list ap) {
54-
const size_t buf_len = 64;
52+
static void __attribute__((format(printf, 6, 0)))
53+
qrexec_logv(__attribute__((unused)) int level, int errnoval,
54+
const char *file, int line,
55+
const char *func, const char *fmt, va_list ap) {
56+
#define buf_len 64
5557
char buf[buf_len];
5658
char *err;
5759
int _errno = errno;
@@ -65,6 +67,7 @@ static void qrexec_logv(__attribute__((unused)) int level, int errnoval,
6567
fprintf(stderr, "\n");
6668
fflush(stderr);
6769
errno = _errno;
70+
#undef buf_len
6871
}
6972

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

0 commit comments

Comments
 (0)