wg: tighten up strtoul parsing

Reported-by: Cedric Buxin <cedric.buxin@izri.org>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>

diff --git a/src/config.c b/src/config.c
index 540b800d8eaa4751c064ea9c890586d168519a91..84038c836de9c86493e35b846f00a718c266d655 100644 (file)
--- a/src/config.c
+++ b/src/config.c
@@ -83,16 +83,22 @@ static inline bool parse_fwmark(uint32_t *fwmark, uint32_t *flags, const char *v
                return true;
        }
 
-       if (value[0] == '0' && value[1] == 'x') {
-               value += 2;
+       if (!isdigit(value[0]))
+               goto err;
+
+       if (strlen(value) > 2 && value[0] == '0' && value[1] == 'x')
                base = 16;
-       }
+
        ret = strtoul(value, &end, base);
-       if (!*value || *end || ret > UINT32_MAX)
-               return false;
+       if (*end || ret > UINT32_MAX)
+               goto err;
+
        *fwmark = ret;
        *flags |= WGDEVICE_HAS_FWMARK;
        return true;
+err:
+       fprintf(stderr, "Fwmark is neither 0/off nor 0-0xffffffff: `%s'\n", value);
+       return false;
 }
 
 static inline bool parse_key(uint8_t key[static WG_KEY_LEN], const char *value)
@@ -206,22 +212,26 @@ static inline bool parse_persistent_keepalive(uint16_t *interval, uint32_t *flag
                return true;
        }
 
+       if (!isdigit(value[0]))
+               goto err;
+
        ret = strtoul(value, &end, 10);
-       if (!*value || *value == '-' || *end || ret > 65535) {
-               fprintf(stderr, "The persistent keepalive interval must be 0/off or 1-65535. Found: `%s'\n", value);
-               return false;
-       }
+       if (*end || ret > 65535)
+               goto err;
 
        *interval = (uint16_t)ret;
        *flags |= WGPEER_HAS_PERSISTENT_KEEPALIVE_INTERVAL;
        return true;
+err:
+       fprintf(stderr, "Persistent keepalive interval is neither 0/off nor 1-65535: `%s'\n", value);
+       return false;
 }
 
 
 static inline bool parse_allowedips(struct wgpeer *peer, struct wgallowedip **last_allowedip, const char *value)
 {
        struct wgallowedip *allowedip = *last_allowedip, *new_allowedip;
-       char *mask, *mutable = strdup(value), *sep;
+       char *mask, *mutable = strdup(value), *sep, *saved_entry;
 
        if (!mutable) {
                perror("strdup");
@@ -234,41 +244,57 @@ static inline bool parse_allowedips(struct wgpeer *peer, struct wgallowedip **la
        }
        sep = mutable;
        while ((mask = strsep(&sep, ","))) {
-               unsigned long cidr = ULONG_MAX;
-               char *end, *ip = strsep(&mask, "/");
+               unsigned long cidr;
+               char *end, *ip;
+
+               saved_entry = strdup(mask);
+               ip = strsep(&mask, "/");
 
                new_allowedip = calloc(1, sizeof(struct wgallowedip));
                if (!new_allowedip) {
                        perror("calloc");
+                       free(saved_entry);
                        free(mutable);
                        return false;
                }
-               if (allowedip)
-                       allowedip->next_allowedip = new_allowedip;
-               else
-                       peer->first_allowedip = new_allowedip;
-               allowedip = new_allowedip;
 
-               if (!parse_ip(allowedip, ip)) {
+               if (!parse_ip(new_allowedip, ip)) {
+                       free(saved_entry);
                        free(mutable);
                        return false;
                }
-               if (mask && *mask) {
+
+               if (mask) {
+                       if (!isdigit(mask[0]))
+                               goto err;
                        cidr = strtoul(mask, &end, 10);
-                       if (*end)
-                               cidr = ULONG_MAX;
-               }
-               if (allowedip->family == AF_INET)
-                       cidr = cidr > 32 ? 32 : cidr;
-               else if (allowedip->family == AF_INET6)
-                       cidr = cidr > 128 ? 128 : cidr;
+                       if (*end || (cidr > 32 && new_allowedip->family == AF_INET) || (cidr > 128 && new_allowedip->family == AF_INET6))
+                               goto err;
+               } else if (new_allowedip->family == AF_INET)
+                       cidr = 32;
+               else if (new_allowedip->family == AF_INET6)
+                       cidr = 128;
                else
-                       continue;
-               allowedip->cidr = cidr;
+                       goto err;
+               new_allowedip->cidr = cidr;
+
+               if (allowedip)
+                       allowedip->next_allowedip = new_allowedip;
+               else
+                       peer->first_allowedip = new_allowedip;
+               allowedip = new_allowedip;
+               free(saved_entry);
        }
        free(mutable);
        *last_allowedip = allowedip;
        return true;
+
+err:
+       free(new_allowedip);
+       free(mutable);
+       fprintf(stderr, "AllowedIP is not in the correct format: `%s'\n", saved_entry);
+       free(saved_entry);
+       return false;
 }
 
 static bool process_line(struct config_ctx *ctx, const char *line)

diff --git a/src/ipc.c b/src/ipc.c
index 0f87d2472d0107581a6321d856bc2b1ccd75e1f6..2d2428746c46cd32b0e824121db7cf4b97824066 100644 (file)
--- a/src/ipc.c
+++ b/src/ipc.c
@@ -16,6 +16,7 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <ctype.h>
 #include <unistd.h>
 #include <time.h>
 #include <dirent.h>
@@ -280,7 +281,7 @@ static int userspace_set_device(struct wgdevice *dev)
 #define NUM(max) ({ \
        unsigned long long num; \
        char *end; \
-       if (!strlen(value)) \
+       if (!isdigit(value[0])) \
                break; \
        num = strtoull(value, &end, 10); \
        if (*end || num > max) \
@@ -398,11 +399,10 @@ static int userspace_get_device(struct wgdevice **out, const char *interface)
                        peer->flags |= WGPEER_HAS_PERSISTENT_KEEPALIVE_INTERVAL;
                } else if (peer && !strcmp(key, "allowed_ip")) {
                        struct wgallowedip *new_allowedip;
-                       char *end, *cidr = strchr(value, '/');
+                       char *end, *mask = value, *ip = strsep(&mask, "/");
 
-                       if (!cidr || strlen(cidr) <= 1)
+                       if (!mask || !isdigit(mask[0]))
                                break;
-                       *cidr++ = '\0';
                        new_allowedip = calloc(1, sizeof(struct wgallowedip));
                        if (!new_allowedip) {
                                ret = -ENOMEM;
@@ -414,14 +414,14 @@ static int userspace_get_device(struct wgdevice **out, const char *interface)
                                peer->first_allowedip = new_allowedip;
                        allowedip = new_allowedip;
                        allowedip->family = AF_UNSPEC;
-                       if (strchr(value, ':')) {
-                               if (inet_pton(AF_INET6, value, &allowedip->ip6) == 1)
+                       if (strchr(ip, ':')) {
+                               if (inet_pton(AF_INET6, ip, &allowedip->ip6) == 1)
                                        allowedip->family = AF_INET6;
                        } else {
-                               if (inet_pton(AF_INET, value, &allowedip->ip4) == 1)
+                               if (inet_pton(AF_INET, ip, &allowedip->ip4) == 1)
                                        allowedip->family = AF_INET;
                        }
-                       allowedip->cidr = strtoul(cidr, &end, 10);
+                       allowedip->cidr = strtoul(mask, &end, 10);
                        if (*end || allowedip->family == AF_UNSPEC || (allowedip->family == AF_INET6 && allowedip->cidr > 128) || (allowedip->family == AF_INET && allowedip->cidr > 32))
                                break;
                } else if (peer && !strcmp(key, "last_handshake_time_sec"))