Fix coverity-1610057

Coverity caught a error in a recent change, in which atoi was used to
assign a value to two size_t variables, and then checked them for being
>= 0, which will always be true.

given that atoi returns an undefined value (usually zero) in the event
of a failure, theres no good way to check the return value of atoi for
validitiy.

Instead use OPENSSL_strtoul and confirm both that the translation
passed, and that the endptr value is at the NULL terminator (indicating
that the entire string was consumed)

Fixes openssl/private#552

Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24861)

diff --git a/ssl/ssl_conf.c b/ssl/ssl_conf.c
index 0deae1604f7b65cb440e11adba86a72a7ee7d1e1..13c7e0ff78731bb229ec5a4862d703d3e86cbc3d 100644 (file)
--- a/ssl/ssl_conf.c
+++ b/ssl/ssl_conf.c
@@ -658,8 +658,9 @@ static int cmd_DHParameters(SSL_CONF_CTX *cctx, const char *value)
 static int cmd_RecordPadding(SSL_CONF_CTX *cctx, const char *value)
 {
     int rv = 0;
-    size_t block_padding = 0, hs_padding = 0;
+    unsigned long block_padding = 0, hs_padding = 0;
     char *commap = NULL, *copy = NULL;
+    char *endptr = NULL;
 
     copy = OPENSSL_strdup(value);
     if (copy == NULL)
@@ -671,9 +672,11 @@ static int cmd_RecordPadding(SSL_CONF_CTX *cctx, const char *value)
             OPENSSL_free(copy);
             return 0;
         }
-        hs_padding = (size_t) atoi(commap + 1);
+        if (!OPENSSL_strtoul(commap + 1, &endptr, 0, &hs_padding))
+            return 0;
     }
-    block_padding = (size_t) atoi(copy);
+    if (!OPENSSL_strtoul(copy, &endptr, 0, &block_padding))
+        return 0;
     if (commap == NULL)
         hs_padding = block_padding;
     OPENSSL_free(copy);
@@ -683,10 +686,11 @@ static int cmd_RecordPadding(SSL_CONF_CTX *cctx, const char *value)
      */
     if (block_padding >= 0 || hs_padding >= 0) {
         if (cctx->ctx)
-            rv = SSL_CTX_set_block_padding_ex(cctx->ctx, block_padding,
-                                              hs_padding);
+            rv = SSL_CTX_set_block_padding_ex(cctx->ctx, (size_t)block_padding,
+                                              (size_t)hs_padding);
         if (cctx->ssl)
-            rv = SSL_set_block_padding_ex(cctx->ssl, block_padding, hs_padding);
+            rv = SSL_set_block_padding_ex(cctx->ssl, (size_t)block_padding,
+                                          (size_t)hs_padding);
     }
     return rv;
 }