Skip to content

Resolve buffer overflow #34

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jserv merged 1 commit into from
Apr 1, 2026
Merged

Resolve buffer overflow #34

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 13 additions & 38 deletions src/net-slirp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
#include <errno.h>
#include <fcntl.h>
#include <limits.h>
#include <net/if.h>
#include <net/route.h>
#include <poll.h>
#include <pthread.h>
#include <stdio.h>
Expand Down Expand Up @@ -1037,18 +1039,9 @@ int kbox_net_configure(const struct kbox_sysnrs *sysnrs)
return -1;
Copy link
Copy Markdown
Contributor

@jserv jserv Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Every struct that crosses the lkl_syscall6 boundary in this codebase has a _Static_assert for size and critical offsets (see kbox_lkl_stat and kbox_open_how in lkl-wrap.h). This one should too:

_Static_assert(sizeof(struct ifreq) == 40,
               "struct ifreq must be 40 bytes (64-bit Linux ABI)");

}

struct {
char ifr_name[16];
union {
short ifr_flags;
struct {
unsigned short sin_family;
unsigned short sin_port;
unsigned int sin_addr;
char sin_zero[8];
} ifr_addr;
};
} ifr;
_Static_assert(sizeof(struct ifreq) == 40,
"struct ifreq must be 40 bytes (64-bit Linux ABI)");
struct ifreq ifr;
memset(&ifr, 0, sizeof(ifr));
snprintf(ifr.ifr_name, sizeof(ifr.ifr_name), "eth%d", lkl_netdev_id);

Expand All @@ -1064,8 +1057,9 @@ int kbox_net_configure(const struct kbox_sysnrs *sysnrs)

/* 2. Set IP address via SIOCSIFADDR. */
Copy link
Copy Markdown
Contributor

@jserv jserv Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sa_data[2] is a magic offset that encodes knowledge of sockaddr_in layout without naming it. The original code had sin_family / sin_addr which were self-documenting. Cast to sockaddr_in * instead:

struct sockaddr_in *addr = (struct sockaddr_in *) &ifr.ifr_addr;
addr->sin_family = AF_INET;
inet_pton(AF_INET, GUEST_IP_STR, &addr->sin_addr);

Same for the netmask block below.

memset(&ifr.ifr_addr, 0, sizeof(ifr.ifr_addr));
ifr.ifr_addr.sin_family = AF_INET;
inet_pton(AF_INET, GUEST_IP_STR, &ifr.ifr_addr.sin_addr);
struct sockaddr_in *addr = (struct sockaddr_in *) &ifr.ifr_addr;
addr->sin_family = AF_INET;
inet_pton(AF_INET, GUEST_IP_STR, &addr->sin_addr);
ret =
lkl_syscall6(LKL_NR_IOCTL, sock, LKL_SIOCSIFADDR, (long) &ifr, 0, 0, 0);
if (ret < 0) {
Expand All @@ -1076,8 +1070,8 @@ int kbox_net_configure(const struct kbox_sysnrs *sysnrs)

/* 3. Set netmask via SIOCSIFNETMASK. */
memset(&ifr.ifr_addr, 0, sizeof(ifr.ifr_addr));
ifr.ifr_addr.sin_family = AF_INET;
inet_pton(AF_INET, "255.255.255.0", &ifr.ifr_addr.sin_addr);
addr->sin_family = AF_INET;
inet_pton(AF_INET, "255.255.255.0", &addr->sin_addr);
ret = lkl_syscall6(LKL_NR_IOCTL, sock, LKL_SIOCSIFNETMASK, (long) &ifr, 0,
0, 0);
if (ret < 0) {
Expand All @@ -1093,28 +1087,9 @@ int kbox_net_configure(const struct kbox_sysnrs *sysnrs)
__atomic_store_n(&net_ready, 1, __ATOMIC_RELEASE);
Copy link
Copy Markdown
Contributor

@jserv jserv Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as ifreq -- add a _Static_assert for sizeof(struct rtentry). The old hand-rolled struct was missing rt_window and rt_irtt, so the size difference is the whole reason this needed fixing. Pin it so it cannot silently drift.


/* 4. Set default gateway via SIOCADDRT. */
struct {
unsigned long rt_pad1;
struct {
unsigned short sa_family;
char sa_data[14];
} rt_dst;
struct {
unsigned short sa_family;
char sa_data[14];
} rt_gateway;
struct {
unsigned short sa_family;
char sa_data[14];
} rt_genmask;
unsigned short rt_flags;
short rt_pad2;
unsigned long rt_pad3;
void *rt_pad4;
short rt_metric;
char *rt_dev;
unsigned long rt_mtu;
} rt;
_Static_assert(sizeof(struct rtentry) == 120,
"struct rtentry must be 40 bytes (kernel ABI)");
struct rtentry rt;
memset(&rt, 0, sizeof(rt));
rt.rt_dst.sa_family = AF_INET;
rt.rt_genmask.sa_family = AF_INET;
Expand Down
Loading