Attention is currently required from: plaisthos.

Hello plaisthos,

I'd like you to do a code review.
Please visit

    http://gerrit.openvpn.net/c/openvpn/+/1490?usp=email

to review the following change.


Change subject: Change signedness of constants to avoid sign-compare issues
......................................................................

Change signedness of constants to avoid sign-compare issues

Improve conditionals where we want to ensure that
a signed int is greater than a unsigned constant int
by forcing the comparison to be signed (instead
of the default unsigned comparison).
This avoids the potential corner case where the
integer might falsely compare as bigger due to being
negative.

Even if we know the comparison to be safe already, we
still need the cast to suppress -Wsign-compare errors.

Change-Id: I68c8ebb593262116b42b4c749d4aed3078f8499b
Signed-off-by: Frank Lichtenheld <[email protected]>
---
M src/openvpn/clinat.c
M src/openvpn/forward.c
M src/openvpn/networking_sitnl.c
M src/openvpn/proto.c
M src/openvpn/push.c
M src/openvpn/ssl_pkt.c
M src/openvpn/tls_crypt.c
M src/openvpn/tun.h
M src/openvpn/vlan.c
9 files changed, 22 insertions(+), 22 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/90/1490/1

diff --git a/src/openvpn/clinat.c b/src/openvpn/clinat.c
index f671fee..fb8e497 100644
--- a/src/openvpn/clinat.c
+++ b/src/openvpn/clinat.c
@@ -249,14 +249,14 @@

         if (h->ip.protocol == OPENVPN_IPPROTO_TCP)
         {
-            if (BLEN(ipbuf) >= sizeof(struct openvpn_iphdr) + sizeof(struct 
openvpn_tcphdr))
+            if (BLEN(ipbuf) >= (int)(sizeof(struct openvpn_iphdr) + 
sizeof(struct openvpn_tcphdr)))
             {
                 ADJUST_CHECKSUM(accumulate, h->u.tcp.check);
             }
         }
         else if (h->ip.protocol == OPENVPN_IPPROTO_UDP)
         {
-            if (BLEN(ipbuf) >= sizeof(struct openvpn_iphdr) + sizeof(struct 
openvpn_udphdr))
+            if (BLEN(ipbuf) >= (int)(sizeof(struct openvpn_iphdr) + 
sizeof(struct openvpn_udphdr)))
             {
                 ADJUST_CHECKSUM(accumulate, h->u.udp.check);
             }
diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
index 5004e35..edb91fe 100644
--- a/src/openvpn/forward.c
+++ b/src/openvpn/forward.c
@@ -1386,7 +1386,7 @@
         struct openvpn_iphdr *pip = (struct openvpn_iphdr *)(BPTR(buf) + 
ip_hdr_offset);
         const int ip_hlen = OPENVPN_IPH_GET_LEN(pip->version_len);
         /* Reject malformed or truncated headers */
-        if (ip_hlen < sizeof(struct openvpn_iphdr)
+        if (ip_hlen < (int)sizeof(struct openvpn_iphdr)
             || BLEN(buf) < (int)(ip_hdr_offset + ip_hlen + sizeof(uint16_t) * 
2))
         {
             return;
diff --git a/src/openvpn/networking_sitnl.c b/src/openvpn/networking_sitnl.c
index a5b2905..b4232f6 100644
--- a/src/openvpn/networking_sitnl.c
+++ b/src/openvpn/networking_sitnl.c
@@ -388,7 +388,7 @@
             if (h->nlmsg_type == NLMSG_ERROR)
             {
                 err = (struct nlmsgerr *)NLMSG_DATA(h);
-                if (rem_len < sizeof(struct nlmsgerr))
+                if (rem_len < (int)sizeof(struct nlmsgerr))
                 {
                     msg(M_WARN, "%s: ERROR truncated", __func__);
                     ret = -EIO;
diff --git a/src/openvpn/proto.c b/src/openvpn/proto.c
index a55a2ae..8a01d00 100644
--- a/src/openvpn/proto.c
+++ b/src/openvpn/proto.c
@@ -45,7 +45,7 @@
     verify_align_4(buf);
     if (tunnel_type == DEV_TYPE_TUN)
     {
-        if (BLEN(buf) < sizeof(struct openvpn_iphdr))
+        if (BLEN(buf) < (int)sizeof(struct openvpn_iphdr))
         {
             return false;
         }
@@ -54,7 +54,7 @@
     else if (tunnel_type == DEV_TYPE_TAP)
     {
         const struct openvpn_ethhdr *eh;
-        if (BLEN(buf) < (sizeof(struct openvpn_ethhdr) + sizeof(struct 
openvpn_iphdr)))
+        if (BLEN(buf) < (int)(sizeof(struct openvpn_ethhdr) + sizeof(struct 
openvpn_iphdr)))
         {
             return false;
         }
@@ -70,7 +70,7 @@
         if (proto == htons(OPENVPN_ETH_P_8021Q))
         {
             const struct openvpn_8021qhdr *evh;
-            if (BLEN(buf) < (sizeof(struct openvpn_ethhdr) + sizeof(struct 
openvpn_iphdr)))
+            if (BLEN(buf) < (int)(sizeof(struct openvpn_ethhdr) + 
sizeof(struct openvpn_iphdr)))
             {
                 return false;
             }
diff --git a/src/openvpn/push.c b/src/openvpn/push.c
index 25df48d..e328d9b 100644
--- a/src/openvpn/push.c
+++ b/src/openvpn/push.c
@@ -829,7 +829,7 @@
         buf_printf(&buf, ",push-continuation 1");
     }

-    if (BLEN(&buf) > sizeof(push_reply_cmd) - 1)
+    if (BLEN(&buf) >= (int)sizeof(push_reply_cmd))
     {
         const bool status = send_control_channel_string(c, BSTR(&buf), D_PUSH);
         if (!status)
diff --git a/src/openvpn/ssl_pkt.c b/src/openvpn/ssl_pkt.c
index 4e97ae4..90ac5ab 100644
--- a/src/openvpn/ssl_pkt.c
+++ b/src/openvpn/ssl_pkt.c
@@ -298,7 +298,7 @@
 {
     struct gc_arena gc = gc_new();
     /* A packet needs to have at least an opcode and session id */
-    if (buf->len < (1 + SID_SIZE))
+    if (buf->len < (int)(1 + SID_SIZE))
     {
         dmsg(D_TLS_STATE_ERRORS, "TLS State Error: Too short packet (length  
%d) received from %s",
              buf->len, print_link_socket_actual(from, &gc));
diff --git a/src/openvpn/tls_crypt.c b/src/openvpn/tls_crypt.c
index 2b00e1c..730c20d 100644
--- a/src/openvpn/tls_crypt.c
+++ b/src/openvpn/tls_crypt.c
@@ -222,7 +222,7 @@

     dmsg(D_PACKET_CONTENT, "TLS-CRYPT UNWRAP FROM: %s", format_hex(BPTR(src), 
BLEN(src), 80, &gc));

-    if (buf_len(src) < TLS_CRYPT_OFF_CT)
+    if (buf_len(src) < (int)TLS_CRYPT_OFF_CT)
     {
         CRYPT_ERROR("packet too short");
     }
@@ -441,7 +441,7 @@
     uint16_t net_len = 0;
     const uint8_t *tag = BPTR(&wrapped_client_key);

-    if (BLEN(&wrapped_client_key) < sizeof(net_len))
+    if (BLEN(&wrapped_client_key) < (int)sizeof(net_len))
     {
         CRYPT_ERROR("failed to read length");
     }
@@ -496,7 +496,7 @@
                              "a different tls-crypt-v2 server key)");
     }

-    if (buf_len(&plaintext) < sizeof(client_key->keys))
+    if (buf_len(&plaintext) < (int)sizeof(client_key->keys))
     {
         CRYPT_ERROR("failed to read client key");
     }
@@ -523,7 +523,7 @@
 static bool
 tls_crypt_v2_check_client_key_age(const struct tls_wrap_ctx *ctx, int max_days)
 {
-    if (ctx->tls_crypt_v2_metadata.len < 1 + sizeof(int64_t))
+    if (ctx->tls_crypt_v2_metadata.len < 1 + (int)sizeof(int64_t))
     {
         msg(M_WARN, "ERROR: Client key metadata is too small to contain a 
timestamp.");
         return false;
@@ -619,7 +619,7 @@
     struct buffer wrapped_client_key = *buf;
     uint16_t net_len = 0;

-    if (BLEN(&wrapped_client_key) < sizeof(net_len))
+    if (BLEN(&wrapped_client_key) < (int)sizeof(net_len))
     {
         msg(D_TLS_ERRORS, "Can not read tls-crypt-v2 client key length");
         return false;
diff --git a/src/openvpn/tun.h b/src/openvpn/tun.h
index 34f4929..a226e37 100644
--- a/src/openvpn/tun.h
+++ b/src/openvpn/tun.h
@@ -510,14 +510,14 @@

     if (OPENVPN_IPH_GET_VER(ih->version_len) == 4)
     {
-        if (BLEN(buf) < sizeof(struct openvpn_iphdr))
+        if (BLEN(buf) < (int)sizeof(struct openvpn_iphdr))
         {
             return false;
         }
     }
     else if (OPENVPN_IPH_GET_VER(ih->version_len) == 6)
     {
-        if (BLEN(buf) < sizeof(struct openvpn_ipv6hdr))
+        if (BLEN(buf) < (int)sizeof(struct openvpn_ipv6hdr))
         {
             return false;
         }
diff --git a/src/openvpn/vlan.c b/src/openvpn/vlan.c
index 85a54eb..3b8e4a8 100644
--- a/src/openvpn/vlan.c
+++ b/src/openvpn/vlan.c
@@ -85,7 +85,7 @@
     uint16_t vid;

     /* assume untagged frame */
-    if (BLEN(buf) < sizeof(*ethhdr))
+    if (BLEN(buf) < (int)sizeof(*ethhdr))
     {
         goto drop;
     }
@@ -109,7 +109,7 @@
     }

     /* tagged frame */
-    if (BLEN(buf) < sizeof(*vlanhdr))
+    if (BLEN(buf) < (int)sizeof(*vlanhdr))
     {
         goto drop;
     }
@@ -184,7 +184,7 @@
     const struct openvpn_ethhdr *ethhdr;
     struct openvpn_8021qhdr *vlanhdr;

-    if (BLEN(buf) < sizeof(*ethhdr))
+    if (BLEN(buf) < (int)sizeof(*ethhdr))
     {
         goto drop;
     }
@@ -197,7 +197,7 @@
          */

         /* Frame too small for header type? */
-        if (BLEN(buf) < sizeof(*vlanhdr))
+        if (BLEN(buf) < (int)sizeof(*vlanhdr))
         {
             goto drop;
         }
@@ -216,7 +216,7 @@
         /* Untagged frame. */

         /* Not enough head room for VLAN tag? */
-        if (buf_reverse_capacity(buf) < SIZE_ETH_TO_8021Q_HDR)
+        if (buf_reverse_capacity(buf) < (int)SIZE_ETH_TO_8021Q_HDR)
         {
             goto drop;
         }
@@ -263,7 +263,7 @@
     const struct openvpn_8021qhdr *vlanhdr;
     uint16_t vid;

-    if (BLEN(buf) < sizeof(struct openvpn_8021qhdr))
+    if (BLEN(buf) < (int)sizeof(struct openvpn_8021qhdr))
     {
         /* frame too small to be VLAN-tagged */
         return false;

--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1490?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings?usp=email

Gerrit-MessageType: newchange
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I68c8ebb593262116b42b4c749d4aed3078f8499b
Gerrit-Change-Number: 1490
Gerrit-PatchSet: 1
Gerrit-Owner: flichtenheld <[email protected]>
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: plaisthos <[email protected]>
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Reply via email to