Skip to content

Fix memory-safety and hang bugs in native packet engine #751

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
kasnder merged 1 commit into from
Aug 22, 2026
Merged

Fix memory-safety and hang bugs in native packet engine #751

Show file tree
Hide file tree
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
2 changes: 1 addition & 1 deletion app/src/main/jni/netguard/dhcp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ int check_dhcp(const struct arguments *args, const struct udp_session *u,
memset(&response->giaddr, 0, sizeof(response->giaddr));

// https://tools.ietf.org/html/rfc2132
uint8_t *options = (uint8_t *) (response + sizeof(struct dhcp_packet));
uint8_t *options = (uint8_t *) (response + 1);

int idx = 0;
*(options + idx++) = 53; // Message type
Expand Down
2 changes: 1 addition & 1 deletion app/src/main/jni/netguard/icmp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ void check_icmp_socket(const struct arguments *args, const struct epoll_event *e
memset(&pseudo, 0, sizeof(struct ip6_hdr_pseudo));
memcpy(&pseudo.ip6ph_src, &s->icmp.daddr.ip6, 16);
memcpy(&pseudo.ip6ph_dst, &s->icmp.saddr.ip6, 16);
pseudo.ip6ph_len = bytes - sizeof(struct ip6_hdr);
pseudo.ip6ph_len = htonl((uint32_t) bytes);
pseudo.ip6ph_nxt = IPPROTO_ICMPV6;
csum = calc_checksum(
0, (uint8_t *) &pseudo, sizeof(struct ip6_hdr_pseudo));
Expand Down
10 changes: 10 additions & 0 deletions app/src/main/jni/netguard/ip.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,11 @@ void handle_ip(const struct arguments *args,
uint8_t ipoptlen = (uint8_t) ((ip4hdr->ihl - 5) * 4);
payload = (uint8_t *) (pkt + sizeof(struct iphdr) + ipoptlen);

if (ip4hdr->ihl < 5 || sizeof(struct iphdr) + ipoptlen > length) {
log_android(ANDROID_LOG_WARN, "IP4 invalid header length");
return;
}

if (ntohs(ip4hdr->tot_len) != length) {
log_android(ANDROID_LOG_ERROR, "Invalid length %u header length %u",
length, ntohs(ip4hdr->tot_len));
Expand Down Expand Up @@ -563,6 +568,11 @@ void handle_ip(const struct arguments *args,
const struct ip6_hdr *ip6 = (struct ip6_hdr *) pkt;
const struct tcphdr *tcphdr = (struct tcphdr *) payload;
const uint8_t tcpoptlen = (uint8_t) ((tcphdr->doff - 5) * 4);
if (tcphdr->doff < 5 ||
sizeof(struct tcphdr) + tcpoptlen > (size_t) (length - (payload - pkt))) {
log_android(ANDROID_LOG_WARN, "TCP invalid data offset");
return;
}
const uint8_t *tcpoptions = payload + sizeof(struct tcphdr);
const uint8_t *data = payload + sizeof(struct tcphdr) + tcpoptlen;
const uint16_t datalen = (const uint16_t) (length - (data - pkt));
Expand Down
17 changes: 12 additions & 5 deletions app/src/main/jni/netguard/netguard.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,8 @@ void JNI_OnUnload(JavaVM *vm, void *reserved) {
JNIEXPORT jlong JNICALL
Java_eu_faircode_netguard_ServiceSinkhole_jni_1init(
JNIEnv *env, jobject instance, jint sdk) {
srand((unsigned int) (time(NULL) ^ getpid()));

// Resolve the routing policy now: the packet path must never pay a dlopen,
// and a failure should be logged while there is still something to read it.
policy_ensure();
Expand Down Expand Up @@ -368,10 +370,15 @@ Java_eu_faircode_netguard_ServiceSinkhole_jni_1socks5(JNIEnv *env, jobject insta
ng_add_alloc(username, "username");
ng_add_alloc(password, "password");

strcpy(socks5_addr, addr);
if (snprintf(socks5_addr, sizeof(socks5_addr), "%s", addr) >= (int) sizeof(socks5_addr))
log_android(ANDROID_LOG_WARN, "SOCKS5 address truncated");
socks5_port = port;
strcpy(socks5_username, username);
strcpy(socks5_password, password);
if (snprintf(socks5_username, sizeof(socks5_username), "%s", username) >=
(int) sizeof(socks5_username))
log_android(ANDROID_LOG_WARN, "SOCKS5 username truncated");
if (snprintf(socks5_password, sizeof(socks5_password), "%s", password) >=
(int) sizeof(socks5_password))
log_android(ANDROID_LOG_WARN, "SOCKS5 password truncated");

log_android(ANDROID_LOG_WARN, "SOCKS5 %s:%d user=%s",
socks5_addr, socks5_port, socks5_username);
Expand Down Expand Up @@ -584,7 +591,7 @@ void report_exit(const struct arguments *args, int error, const char *fmt, ...)
char line[1024];
va_list argptr;
va_start(argptr, fmt);
vsprintf(line, fmt, argptr);
vsnprintf(line, sizeof(line), fmt, argptr);
jreason = (*args->env)->NewStringUTF(args->env, line);
ng_add_alloc(jreason, "jreason");
va_end(argptr);
Expand All @@ -611,7 +618,7 @@ void report_error(const struct arguments *args, jint error, const char *fmt, ...
char line[1024];
va_list argptr;
va_start(argptr, fmt);
vsprintf(line, fmt, argptr);
vsnprintf(line, sizeof(line), fmt, argptr);
jreason = (*args->env)->NewStringUTF(args->env, line);
ng_add_alloc(jreason, "jreason");
va_end(argptr);
Expand Down
7 changes: 5 additions & 2 deletions app/src/main/jni/netguard/pcap.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,14 @@ void write_pcap(const void *ptr, size_t len) {

if (fsize > pcap_file_size) {
log_android(ANDROID_LOG_WARN, "PCAP truncate @%ld", fsize);
if (ftruncate(fileno(pcap_file), sizeof(struct pcap_hdr_s)))
if (fflush(pcap_file))
log_android(ANDROID_LOG_ERROR, "PCAP fflush error %d: %s",
errno, strerror(errno));
else if (ftruncate(fileno(pcap_file), sizeof(struct pcap_hdr_s)))
log_android(ANDROID_LOG_ERROR, "PCAP ftruncate error %d: %s",
errno, strerror(errno));
else {
if (!lseek(fileno(pcap_file), sizeof(struct pcap_hdr_s), SEEK_SET))
if (fseek(pcap_file, sizeof(struct pcap_hdr_s), SEEK_SET))
log_android(ANDROID_LOG_ERROR, "PCAP ftruncate error %d: %s",
errno, strerror(errno));
}
Expand Down
28 changes: 20 additions & 8 deletions app/src/main/jni/netguard/tcp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -649,6 +649,11 @@ jboolean handle_tcp(const struct arguments *args,
const struct ip6_hdr *ip6 = (struct ip6_hdr *) pkt;
const struct tcphdr *tcphdr = (struct tcphdr *) payload;
const uint8_t tcpoptlen = (uint8_t) ((tcphdr->doff - 5) * 4);
if (tcphdr->doff < 5 ||
sizeof(struct tcphdr) + tcpoptlen > (size_t) (length - (payload - pkt))) {
log_android(ANDROID_LOG_WARN, "TCP invalid data offset");
return 0;
}
const uint8_t *tcpoptions = payload + sizeof(struct tcphdr);
const uint8_t *data = payload + sizeof(struct tcphdr) + tcpoptlen;
const uint16_t datalen = (const uint16_t) (length - (data - pkt));
Expand Down Expand Up @@ -718,23 +723,30 @@ jboolean handle_tcp(const struct arguments *args,
uint8_t *options = (uint8_t *) tcpoptions;
while (optlen > 0) {
uint8_t kind = *options;
uint8_t len = *(options + 1);
if (kind == 0) // End of options list
break;

if (kind == 1) {
optlen--;
options++;
continue;
}

if (optlen < 2)
break;

uint8_t len = *(options + 1);
if (len < 2 || len > optlen)
break;

if (kind == 2 && len == 4)
mss = ntohs(*((uint16_t *) (options + 2)));

else if (kind == 3 && len == 3)
ws = *(options + 2);

if (kind == 1) {
optlen--;
options++;
} else {
optlen -= len;
options += len;
}
optlen -= len;
options += len;
}

// In tethering compatibility mode, clamp the MSS we use for
Expand Down
2 changes: 1 addition & 1 deletion app/src/main/jni/netguard/util.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ void log_android(int prio, const char *fmt, ...) {
char line[1024];
va_list argptr;
va_start(argptr, fmt);
vsprintf(line, fmt, argptr);
vsnprintf(line, sizeof(line), fmt, argptr);
__android_log_print(prio, TAG, "%s", line);
va_end(argptr);
}
Expand Down