Browse code

options: Review use of positive_atoi vs atoi_constrained

Replace where it is useful.

While here also add a missing cast in atoi_constrained.

Change-Id: Id440917f433aab1a7db608ba04fa95ba47c2ddde
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1153
Message-Id: <20251009205951.32301-1-gert@greenie.muc.de>
URL: https://sourceforge.net/p/openvpn/mailman/message/59244617/
Signed-off-by: Gert Doering <gert@greenie.muc.de>

Frank Lichtenheld authored on 2025/10/10 05:59:46
Showing 1 changed files
... ...
@@ -6421,21 +6421,15 @@ add_option(struct options *options, char *p[], bool is_inline, const char *file,
6421 6421
     else if (streq(p[0], "tun-mtu-max") && p[1] && !p[2])
6422 6422
     {
6423 6423
         VERIFY_PERMISSION(OPT_P_MTU | OPT_P_CONNECTION);
6424
-        int max_mtu = positive_atoi(p[1], msglevel);
6425
-        if (max_mtu < 68 || max_mtu > 65536)
6426
-        {
6427
-            msg(msglevel, "--tun-mtu-max value '%s' is invalid", p[1]);
6428
-        }
6429
-        else
6430
-        {
6431
-            options->ce.tun_mtu_max = max_mtu;
6432
-        }
6424
+        atoi_constrained(p[1], &options->ce.tun_mtu_max, p[0], TUN_MTU_MAX_MIN, 65536, msglevel);
6433 6425
     }
6434 6426
     else if (streq(p[0], "tun-mtu-extra") && p[1] && !p[2])
6435 6427
     {
6436 6428
         VERIFY_PERMISSION(OPT_P_MTU | OPT_P_CONNECTION);
6437
-        options->ce.tun_mtu_extra = positive_atoi(p[1], msglevel);
6438
-        options->ce.tun_mtu_extra_defined = true;
6429
+        if (atoi_constrained(p[1], &options->ce.tun_mtu_extra, p[0], 0, 65536, msglevel))
6430
+        {
6431
+            options->ce.tun_mtu_extra_defined = true;
6432
+        }
6439 6433
     }
6440 6434
     else if (streq(p[0], "max-packet-size") && p[1] && !p[2])
6441 6435
     {
... ...
@@ -6467,11 +6461,8 @@ add_option(struct options *options, char *p[], bool is_inline, const char *file,
6467 6467
     else if (streq(p[0], "fragment") && p[1] && !p[3])
6468 6468
     {
6469 6469
         VERIFY_PERMISSION(OPT_P_MTU | OPT_P_CONNECTION);
6470
-        options->ce.fragment = positive_atoi(p[1], msglevel);
6471
-
6472
-        if (options->ce.fragment < 68)
6470
+        if (!atoi_constrained(p[1], &options->ce.fragment, p[0], 68, INT_MAX, msglevel))
6473 6471
         {
6474
-            msg(msglevel, "--fragment needs to be at least 68");
6475 6472
             goto err;
6476 6473
         }
6477 6474
 
... ...
@@ -7145,12 +7136,14 @@ add_option(struct options *options, char *p[], bool is_inline, const char *file,
7145 7145
         VERIFY_PERMISSION(OPT_P_GENERAL | OPT_P_CONNECTION);
7146 7146
         if (p[1])
7147 7147
         {
7148
-            int mssfix = positive_atoi(p[1], msglevel);
7149
-            /* can be 0, but otherwise it needs to be high enough so we can
7150
-             * substract room for headers. */
7151
-            if (mssfix != 0 && (mssfix < TLS_CHANNEL_MTU_MIN || mssfix > UINT16_MAX))
7148
+            int mssfix;
7149
+            if (!atoi_constrained(p[1], &mssfix, p[0], 0, UINT16_MAX, msglevel))
7150
+            {
7151
+                goto err;
7152
+            }
7153
+            if (mssfix != 0 && mssfix < TLS_CHANNEL_MTU_MIN)
7152 7154
             {
7153
-                msg(msglevel, "--mssfix value '%s' is invalid", p[1]);
7155
+                msg(msglevel, "mssfix needs to be >= %d, not %d", TLS_CHANNEL_MTU_MIN, mssfix);
7154 7156
                 goto err;
7155 7157
             }
7156 7158
 
... ...
@@ -7403,7 +7396,7 @@ add_option(struct options *options, char *p[], bool is_inline, const char *file,
7403 7403
     else if (streq(p[0], "max-routes-per-client") && p[1] && !p[2])
7404 7404
     {
7405 7405
         VERIFY_PERMISSION(OPT_P_INHERIT);
7406
-        options->max_routes_per_client = max_int(positive_atoi(p[1], msglevel), 1);
7406
+        atoi_constrained(p[1], &options->max_routes_per_client, p[0], 1, INT_MAX, msglevel);
7407 7407
     }
7408 7408
     else if (streq(p[0], "client-cert-not-required") && !p[1])
7409 7409
     {
... ...
@@ -9222,8 +9215,6 @@ add_option(struct options *options, char *p[], bool is_inline, const char *file,
9222 9222
     }
9223 9223
     else if (streq(p[0], "keying-material-exporter") && p[1] && p[2])
9224 9224
     {
9225
-        int ekm_length = positive_atoi(p[2], msglevel);
9226
-
9227 9225
         VERIFY_PERMISSION(OPT_P_GENERAL);
9228 9226
 
9229 9227
         if (strncmp(p[1], "EXPORTER", 8))
... ...
@@ -9237,14 +9228,14 @@ add_option(struct options *options, char *p[], bool is_inline, const char *file,
9237 9237
             msg(msglevel,
9238 9238
                 "Keying material exporter label must not be '" EXPORT_KEY_DATA_LABEL "'.");
9239 9239
         }
9240
-        if (ekm_length < 16 || ekm_length > 4095)
9240
+
9241
+        if (!atoi_constrained(p[2], &options->keying_material_exporter_length,
9242
+                              p[0], 16, 4095, msglevel))
9241 9243
         {
9242
-            msg(msglevel, "Invalid keying material exporter length");
9243 9244
             goto err;
9244 9245
         }
9245 9246
 
9246 9247
         options->keying_material_exporter_label = p[1];
9247
-        options->keying_material_exporter_length = ekm_length;
9248 9248
     }
9249 9249
     else if (streq(p[0], "allow-recursive-routing") && !p[1])
9250 9250
     {
... ...
@@ -9279,15 +9270,14 @@ add_option(struct options *options, char *p[], bool is_inline, const char *file,
9279 9279
     }
9280 9280
     else if (streq(p[0], "vlan-pvid") && p[1] && !p[2])
9281 9281
     {
9282
+        int vlan_pvid;
9282 9283
         VERIFY_PERMISSION(OPT_P_GENERAL | OPT_P_INSTANCE);
9283
-        options->vlan_pvid = positive_atoi(p[1], msglevel);
9284
-        if (options->vlan_pvid < OPENVPN_8021Q_MIN_VID
9285
-            || options->vlan_pvid > OPENVPN_8021Q_MAX_VID)
9284
+        if (!atoi_constrained(p[1], &vlan_pvid, p[0],
9285
+                              OPENVPN_8021Q_MIN_VID, OPENVPN_8021Q_MAX_VID, msglevel))
9286 9286
         {
9287
-            msg(msglevel, "the parameter of --vlan-pvid parameters must be >= %u and <= %u",
9288
-                OPENVPN_8021Q_MIN_VID, OPENVPN_8021Q_MAX_VID);
9289 9287
             goto err;
9290 9288
         }
9289
+        options->vlan_pvid = (uint16_t)vlan_pvid;
9291 9290
     }
9292 9291
     else
9293 9292
     {