diff mbox series

[Openvpn-devel,v3] Replace strncat with strlcat

Message ID 20261017171508.15923-1-gert@greenie.muc.de
State New
Headers
Series [Openvpn-devel,v3] Replace strncat with strlcat |

Commit Message

Gert Doering Oct. 17, 2026, 5:15 p.m. UTC 
  From: Frank Lichtenheld <frank@lichtenheld.com>

Generally much better API. Not a straight-forward
replacement in many cases, trying to improve the
code generally.

Change-Id: I61e29dd2af9302b240c93735179d43c72e37b335
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Razvan Cojocaru <razvanc@mailbox.org>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1996
---

This change was reviewed on Gerrit and approved by at least one
developer. I request to merge it to master.

Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1996
This mail reflects revision 3 of this Change.

Acked-by according to Gerrit (reflected above):
Razvan Cojocaru <razvanc@mailbox.org>
diff mbox series

Patch

diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c
index a75bd8f..b9cc6a2 100644
--- a/src/openvpn/misc.c
+++ b/src/openvpn/misc.c
@@ -437,12 +437,11 @@ 
                     }
                     else
                     {
-                        if (strlen(up->password) + strlen(response) >= USER_PASS_LEN)
+                        if (strlcat(up->password, response, USER_PASS_LEN) >= USER_PASS_LEN)
                         {
                             msg(M_FATAL,
                                 "ERROR: could not concatenate password/static_response: string too long");
                         }
-                        strncat(up->password, response, USER_PASS_LEN - strlen(up->password) - 1);
                     }
                 }
 #endif /* ifdef ENABLE_MANAGEMENT */
diff --git a/src/openvpn/options_parse.c b/src/openvpn/options_parse.c
index 12934ab..3d893d4 100644
--- a/src/openvpn/options_parse.c
+++ b/src/openvpn/options_parse.c
@@ -244,10 +244,7 @@ 
     else if (is->type == IS_TYPE_BUF)
     {
         bool status = buf_parse(is->u.multiline, '\n', line, size);
-        if ((int)strlen(line) + 1 < size)
-        {
-            strcat(line, "\n");
-        }
+        strlcat(line, "\n", size);
         return status;
     }
     else
diff --git a/src/openvpn/status.c b/src/openvpn/status.c
index c7d375b..981bb23 100644
--- a/src/openvpn/status.c
+++ b/src/openvpn/status.c
@@ -234,14 +234,11 @@ 
 
         if (so->fd >= 0 && !so->errors)
         {
-            strcat(buf, "\n");
-            ssize_t len = strlen(buf);
-            if (len > 0)
+            size_t len = strlcat(buf, "\n", sizeof(buf));
+            ASSERT(len < sizeof(buf));
+            if (write(so->fd, buf, (unsigned int)len) != (ssize_t)len)
             {
-                if (write(so->fd, buf, (unsigned int)len) != len)
-                {
-                    so->errors = true;
-                }
+                so->errors = true;
             }
         }
 
diff --git a/src/openvpn/xkey_helper.c b/src/openvpn/xkey_helper.c
index 1149c7b..6908cbb 100644
--- a/src/openvpn/xkey_helper.c
+++ b/src/openvpn/xkey_helper.c
@@ -157,7 +157,7 @@ 
  * @param tbslen        len of data in dgst
  * @param sigalg        extra signature parameters
  *
- * @return              signature length or -1 on error.
+ * @return              1 on success or 0 on error.
  */
 int
 xkey_management_sign(void *unused, unsigned char *sig, size_t *siglen, const unsigned char *tbs,
@@ -247,7 +247,10 @@ 
 
     if (is_message)
     {
-        strncat(alg_str, ",data=message", sizeof(alg_str) - strlen(alg_str) - 1);
+        if (strlcat(alg_str, ",data=message", sizeof(alg_str)) >= sizeof(alg_str))
+        {
+            return 0;
+        }
     }
 
     dmsg(D_LOW, "xkey management_sign: requesting sig with algorithm <%s>", alg_str);
diff --git a/src/plugins/auth-pam/utils.c b/src/plugins/auth-pam/utils.c
index a4d2d0b..01cc4de 100644
--- a/src/plugins/auth-pam/utils.c
+++ b/src/plugins/auth-pam/utils.c
@@ -47,10 +47,11 @@ 
     }
 
     size_t tosearchlen = strlen(tosearch);
+    size_t searchforlen = strlen(searchfor);
     size_t replacewithlen = strlen(replacewith);
     size_t templen = tosearchlen * replacewithlen;
 
-    if (tosearchlen == 0 || strlen(searchfor) == 0 || replacewithlen == 0)
+    if (tosearchlen == 0 || searchforlen == 0 || replacewithlen == 0)
     {
         return NULL;
     }
@@ -65,23 +66,26 @@ 
 
     /* state: all parameters are valid */
 
-    const char *searching = tosearch;
-
-    char temp[templen + 1];
-    temp[0] = 0;
-
-    const char *scratch = strstr(searching, searchfor);
+    const char *scratch = strstr(tosearch, searchfor);
     if (!scratch)
     {
         return strdup(tosearch);
     }
 
+    const char *searching = tosearch;
+    char temp[templen + 1];
+    memset(temp, 0, sizeof(temp));
+    char *target = temp;
+
     while (scratch)
     {
-        strncat(temp, searching, (size_t)(scratch - searching));
-        strcat(temp, replacewith);
+        size_t skipped = (size_t)(scratch - searching);
+        memcpy(target, searching, skipped);
+        target += skipped;
+        memcpy(target, replacewith, replacewithlen);
+        target += replacewithlen;
 
-        searching = scratch + strlen(searchfor);
+        searching = scratch + searchforlen;
         scratch = strstr(searching, searchfor);
     }
     return strdup(temp);