[Emacs-diffs] master 99be750: Fix glitches in gnutls.c, mostly memory-related

[Paul Eggert]

branch: master
commit 99be75085cec471fa35a811bddaf09fe91fc3452
Author: Paul Eggert <address@hidden>
Commit: Paul Eggert <address@hidden>

    Fix glitches in gnutls.c, mostly memory-related
    
    * gnutls.c: Sort macro definitions by name.
    (fn_gnutls_transport_set_errno): Omit unreachable definition.
    (fn_gnutls_x509_crt_get_signature): Omit unused symbol.
    (gnutls_hex_string): Arg is now unsigned char *, to avoid a cast.
    Prefer ptrdiff_t for sizes.  Check for arithmetic overflow when
    calculating string length.  Use make_uninit_string, to avoid
    copying the string.  Cast the char, not the pointer.
    (gnutls_certificate_details): Use xmalloc and xfree, not malloc
    and free.  Work even for dates past the year 9999.  Use void *
    for buffers, to avoid casts.
---
 src/ChangeLog |   14 ++++++
 src/gnutls.c  |  139 +++++++++++++++++++++++++++------------------------------
 2 files changed, 80 insertions(+), 73 deletions(-)

diff --git a/src/ChangeLog b/src/ChangeLog
index 762ce48..09268d1 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,17 @@
+2014-12-10  Paul Eggert  <address@hidden>
+
+       Fix glitches in gnutls.c, mostly memory-related
+       * gnutls.c: Sort macro definitions by name.
+       (fn_gnutls_transport_set_errno): Omit unreachable definition.
+       (fn_gnutls_x509_crt_get_signature): Omit unused symbol.
+       (gnutls_hex_string): Arg is now unsigned char *, to avoid a cast.
+       Prefer ptrdiff_t for sizes.  Check for arithmetic overflow when
+       calculating string length.  Use make_uninit_string, to avoid
+       copying the string.  Cast the char, not the pointer.
+       (gnutls_certificate_details): Use xmalloc and xfree, not malloc
+       and free.  Work even for dates past the year 9999.  Use void *
+       for buffers, to avoid casts.
+
 2014-12-09  Andy Moreton  <address@hidden>  (tiny change)
 
        * gnutls.c (gnutls_protocol_get_name): Fix a copy/paste typo.
diff --git a/src/gnutls.c b/src/gnutls.c
index aa800be..4de2eaf 100644
--- a/src/gnutls.c
+++ b/src/gnutls.c
@@ -317,65 +317,61 @@ init_gnutls_functions (void)
 #define fn_gnutls_certificate_set_x509_trust_file 
gnutls_certificate_set_x509_trust_file
 #define fn_gnutls_certificate_type_get         gnutls_certificate_type_get
 #define fn_gnutls_certificate_verify_peers2    gnutls_certificate_verify_peers2
+#define fn_gnutls_cipher_get                    gnutls_cipher_get
+#define fn_gnutls_cipher_get_name               gnutls_cipher_get_name
 #define fn_gnutls_credentials_set              gnutls_credentials_set
 #define fn_gnutls_deinit                       gnutls_deinit
-#define fn_gnutls_dh_set_prime_bits            gnutls_dh_set_prime_bits
 #define fn_gnutls_dh_get_prime_bits            gnutls_dh_get_prime_bits
+#define fn_gnutls_dh_set_prime_bits            gnutls_dh_set_prime_bits
 #define fn_gnutls_error_is_fatal               gnutls_error_is_fatal
 #define fn_gnutls_global_init                  gnutls_global_init
-#define fn_gnutls_global_set_log_function      gnutls_global_set_log_function
 #ifdef HAVE_GNUTLS3
 #define fn_gnutls_global_set_audit_log_function        
gnutls_global_set_audit_log_function
 #endif
+#define fn_gnutls_global_set_log_function      gnutls_global_set_log_function
 #define fn_gnutls_global_set_log_level         gnutls_global_set_log_level
 #define fn_gnutls_global_set_mem_functions     gnutls_global_set_mem_functions
 #define fn_gnutls_handshake                    gnutls_handshake
 #define fn_gnutls_init                         gnutls_init
+#define fn_gnutls_kx_get                        gnutls_kx_get
+#define fn_gnutls_kx_get_name                   gnutls_kx_get_name
+#define fn_gnutls_mac_get                       gnutls_mac_get
+#define fn_gnutls_mac_get_name                  gnutls_mac_get_name
+#define fn_gnutls_pk_algorithm_get_name         gnutls_pk_algorithm_get_name
+#define fn_gnutls_pk_bits_to_sec_param          gnutls_pk_bits_to_sec_param
 #define fn_gnutls_priority_set_direct          gnutls_priority_set_direct
+#define fn_gnutls_protocol_get_name             gnutls_protocol_get_name
+#define fn_gnutls_protocol_get_version          gnutls_protocol_get_version
 #define fn_gnutls_record_check_pending         gnutls_record_check_pending
 #define fn_gnutls_record_recv                  gnutls_record_recv
 #define fn_gnutls_record_send                  gnutls_record_send
+#define fn_gnutls_sec_param_get_name            gnutls_sec_param_get_name
+#define fn_gnutls_server_name_set               gnutls_server_name_set
+#define fn_gnutls_sign_get_name                 gnutls_sign_get_name
 #define fn_gnutls_strerror                     gnutls_strerror
-#ifdef WINDOWSNT
-#define fn_gnutls_transport_set_errno          gnutls_transport_set_errno
-#endif
 #define fn_gnutls_transport_set_ptr2           gnutls_transport_set_ptr2
 #define fn_gnutls_x509_crt_check_hostname      gnutls_x509_crt_check_hostname
 #define fn_gnutls_x509_crt_deinit              gnutls_x509_crt_deinit
-#define fn_gnutls_x509_crt_import              gnutls_x509_crt_import
-#define fn_gnutls_x509_crt_init                        gnutls_x509_crt_init
-#define fn_gnutls_x509_crt_get_fingerprint     gnutls_x509_crt_get_fingerprint
-#define fn_gnutls_x509_crt_get_version          gnutls_x509_crt_get_version
-#define fn_gnutls_x509_crt_get_serial           gnutls_x509_crt_get_serial
-#define fn_gnutls_x509_crt_get_issuer_dn        gnutls_x509_crt_get_issuer_dn
 #define fn_gnutls_x509_crt_get_activation_time  
gnutls_x509_crt_get_activation_time
-#define fn_gnutls_x509_crt_get_expiration_time  
gnutls_x509_crt_get_expiration_time
 #define fn_gnutls_x509_crt_get_dn               gnutls_x509_crt_get_dn
-#define fn_gnutls_x509_crt_get_pk_algorithm     
gnutls_x509_crt_get_pk_algorithm
-#define fn_gnutls_pk_algorithm_get_name         gnutls_pk_algorithm_get_name
-#define fn_gnutls_pk_bits_to_sec_param          gnutls_pk_bits_to_sec_param
+#define fn_gnutls_x509_crt_get_expiration_time  
gnutls_x509_crt_get_expiration_time
+#define fn_gnutls_x509_crt_get_fingerprint     gnutls_x509_crt_get_fingerprint
+#define fn_gnutls_x509_crt_get_issuer_dn        gnutls_x509_crt_get_issuer_dn
 #define fn_gnutls_x509_crt_get_issuer_unique_id 
gnutls_x509_crt_get_issuer_unique_id
-#define fn_gnutls_x509_crt_get_subject_unique_id 
gnutls_x509_crt_get_subject_unique_id
-#define fn_gnutls_x509_crt_get_signature_algorithm 
gnutls_x509_crt_get_signature_algorithm
-#define fn_gnutls_x509_crt_get_signature        gnutls_x509_crt_get_signature
 #define fn_gnutls_x509_crt_get_key_id           gnutls_x509_crt_get_key_id
-#define fn_gnutls_sec_param_get_name            gnutls_sec_param_get_name
-#define fn_gnutls_sign_get_name                 gnutls_sign_get_name
-#define fn_gnutls_server_name_set               gnutls_server_name_set
-#define fn_gnutls_kx_get                        gnutls_kx_get
-#define fn_gnutls_kx_get_name                   gnutls_kx_get_name
-#define fn_gnutls_protocol_get_version          gnutls_protocol_get_version
-#define fn_gnutls_protocol_get_name             gnutls_protocol_get_name
-#define fn_gnutls_cipher_get                    gnutls_cipher_get
-#define fn_gnutls_cipher_get_name               gnutls_cipher_get_name
-#define fn_gnutls_mac_get                       gnutls_mac_get
-#define fn_gnutls_mac_get_name                  gnutls_mac_get_name
+#define fn_gnutls_x509_crt_get_pk_algorithm     
gnutls_x509_crt_get_pk_algorithm
+#define fn_gnutls_x509_crt_get_serial           gnutls_x509_crt_get_serial
+#define fn_gnutls_x509_crt_get_signature_algorithm 
gnutls_x509_crt_get_signature_algorithm
+#define fn_gnutls_x509_crt_get_subject_unique_id 
gnutls_x509_crt_get_subject_unique_id
+#define fn_gnutls_x509_crt_get_version          gnutls_x509_crt_get_version
+#define fn_gnutls_x509_crt_import              gnutls_x509_crt_import
+#define fn_gnutls_x509_crt_init                        gnutls_x509_crt_init
 
 #endif /* !WINDOWSNT */
 
 
 #ifdef HAVE_GNUTLS3
-/* Function to log a simple audit message.  */
+/* Log a simple audit message.  */
 static void
 gnutls_audit_log_function (gnutls_session_t session, const char *string)
 {
@@ -386,21 +382,21 @@ gnutls_audit_log_function (gnutls_session_t session, 
const char *string)
 }
 #endif
 
-/* Function to log a simple message.  */
+/* Log a simple message.  */
 static void
 gnutls_log_function (int level, const char *string)
 {
   message ("gnutls.c: [%d] %s", level, string);
 }
 
-/* Function to log a message and a string.  */
+/* Log a message and a string.  */
 static void
 gnutls_log_function2 (int level, const char *string, const char *extra)
 {
   message ("gnutls.c: [%d] %s %s", level, string, extra);
 }
 
-/* Function to log a message and an integer.  */
+/* Log a message and an integer.  */
 static void
 gnutls_log_function2i (int level, const char *string, int extra)
 {
@@ -804,21 +800,21 @@ DEFUN ("gnutls-available-p", Fgnutls_available_p, 
Sgnutls_available_p, 0, 0, 0,
 }
 
 static Lisp_Object
-gnutls_hex_string (char *buf, size_t buf_size, const char *prefix)
+gnutls_hex_string (unsigned char *buf, ptrdiff_t buf_size, const char *prefix)
 {
-  size_t prefix_length = strlen (prefix);
-  char *string = malloc (buf_size * 3 + prefix_length);
-  Lisp_Object ret;
-
+  ptrdiff_t prefix_length = strlen (prefix);
+  if ((STRING_BYTES_BOUND - prefix_length) / 3 < buf_size)
+    string_overflow ();
+  Lisp_Object ret = make_uninit_string (prefix_length + 3 * buf_size
+                                       - (buf_size != 0));
+  char *string = SSDATA (ret);
   strcpy (string, prefix);
 
-  for (int i = 0; i < buf_size; i++)
+  for (ptrdiff_t i = 0; i < buf_size; i++)
     sprintf (string + i * 3 + prefix_length,
             i == buf_size - 1 ? "%02x" : "%02x:",
-            ((unsigned char*) buf)[i]);
+            buf[i]);
 
-  ret = build_string (string);
-  free (string);
   return ret;
 }
 
@@ -842,12 +838,12 @@ gnutls_certificate_details (gnutls_x509_crt_t cert)
   err = fn_gnutls_x509_crt_get_serial (cert, NULL, &buf_size);
   if (err == GNUTLS_E_SHORT_MEMORY_BUFFER)
     {
-      char *serial = malloc (buf_size);
+      void *serial = xmalloc (buf_size);
       err = fn_gnutls_x509_crt_get_serial (cert, serial, &buf_size);
       if (err >= GNUTLS_E_SUCCESS)
        res = nconc2 (res, list2 (intern (":serial-number"),
                                  gnutls_hex_string (serial, buf_size, "")));
-      free (serial);
+      xfree (serial);
     }
 
   /* Issuer. */
@@ -855,28 +851,27 @@ gnutls_certificate_details (gnutls_x509_crt_t cert)
   err = fn_gnutls_x509_crt_get_issuer_dn (cert, NULL, &buf_size);
   if (err == GNUTLS_E_SHORT_MEMORY_BUFFER)
     {
-      char *dn = malloc (buf_size);
+      char *dn = xmalloc (buf_size);
       err = fn_gnutls_x509_crt_get_issuer_dn (cert, dn, &buf_size);
       if (err >= GNUTLS_E_SUCCESS)
        res = nconc2 (res, list2 (intern (":issuer"),
                                  make_string (dn, buf_size)));
-      free (dn);
+      xfree (dn);
     }
 
   /* Validity. */
   {
-    char buf[11];
-    size_t buf_size = sizeof (buf);
+    /* Add 1 to the buffer size, since 1900 is added to tm_year and
+       that might add 1 to the year length.  */
+    char buf[INT_STRLEN_BOUND (int) + 1 + sizeof "-12-31"];
     struct tm t;
     time_t tim = fn_gnutls_x509_crt_get_activation_time (cert);
 
-    if (gmtime_r (&tim, &t) != NULL &&
-       strftime (buf, buf_size, "%Y-%m-%d", &t) != 0)
+    if (gmtime_r (&tim, &t) && strftime (buf, sizeof buf, "%Y-%m-%d", &t))
       res = nconc2 (res, list2 (intern (":valid-from"), build_string (buf)));
 
     tim = fn_gnutls_x509_crt_get_expiration_time (cert);
-    if (gmtime_r (&tim, &t) != NULL &&
-       strftime (buf, buf_size, "%Y-%m-%d", &t) != 0)
+    if (gmtime_r (&tim, &t) && strftime (buf, sizeof buf, "%Y-%m-%d", &t))
       res = nconc2 (res, list2 (intern (":valid-to"), build_string (buf)));
   }
 
@@ -885,12 +880,12 @@ gnutls_certificate_details (gnutls_x509_crt_t cert)
   err = fn_gnutls_x509_crt_get_dn (cert, NULL, &buf_size);
   if (err == GNUTLS_E_SHORT_MEMORY_BUFFER)
     {
-      char *dn = malloc (buf_size);
+      char *dn = xmalloc (buf_size);
       err = fn_gnutls_x509_crt_get_dn (cert, dn, &buf_size);
       if (err >= GNUTLS_E_SUCCESS)
        res = nconc2 (res, list2 (intern (":subject"),
                                  make_string (dn, buf_size)));
-      free (dn);
+      xfree (dn);
     }
 
   /* Versions older than 2.11 doesn't have these four functions. */
@@ -919,24 +914,24 @@ gnutls_certificate_details (gnutls_x509_crt_t cert)
   err = fn_gnutls_x509_crt_get_issuer_unique_id (cert, NULL, &buf_size);
   if (err == GNUTLS_E_SHORT_MEMORY_BUFFER)
     {
-      char *buf = malloc (buf_size);
+      char *buf = xmalloc (buf_size);
       err = fn_gnutls_x509_crt_get_issuer_unique_id (cert, buf, &buf_size);
       if (err >= GNUTLS_E_SUCCESS)
        res = nconc2 (res, list2 (intern (":issuer-unique-id"),
                                  make_string (buf, buf_size)));
-      free (buf);
+      xfree (buf);
     }
 
   buf_size = 0;
   err = fn_gnutls_x509_crt_get_subject_unique_id (cert, NULL, &buf_size);
   if (err == GNUTLS_E_SHORT_MEMORY_BUFFER)
     {
-      char *buf = malloc (buf_size);
+      char *buf = xmalloc (buf_size);
       err = fn_gnutls_x509_crt_get_subject_unique_id (cert, buf, &buf_size);
       if (err >= GNUTLS_E_SUCCESS)
        res = nconc2 (res, list2 (intern (":subject-unique-id"),
                                  make_string (buf, buf_size)));
-      free (buf);
+      xfree (buf);
     }
 #endif
 
@@ -955,13 +950,12 @@ gnutls_certificate_details (gnutls_x509_crt_t cert)
   err = fn_gnutls_x509_crt_get_key_id (cert, 0, NULL, &buf_size);
   if (err == GNUTLS_E_SHORT_MEMORY_BUFFER)
     {
-      unsigned char *buf = malloc (buf_size);
+      void *buf = xmalloc (buf_size);
       err = fn_gnutls_x509_crt_get_key_id (cert, 0, buf, &buf_size);
       if (err >= GNUTLS_E_SUCCESS)
        res = nconc2 (res, list2 (intern (":public-key-id"),
-                                 gnutls_hex_string ((char *)buf,
-                                                    buf_size, "sha1:")));
-      free (buf);
+                                 gnutls_hex_string (buf, buf_size, "sha1:")));
+      xfree (buf);
     }
 
   /* Certificate fingerprint. */
@@ -970,21 +964,20 @@ gnutls_certificate_details (gnutls_x509_crt_t cert)
                                            NULL, &buf_size);
   if (err == GNUTLS_E_SHORT_MEMORY_BUFFER)
     {
-      unsigned char *buf = malloc (buf_size);
+      void *buf = xmalloc (buf_size);
       err = fn_gnutls_x509_crt_get_fingerprint (cert, GNUTLS_DIG_SHA1,
                                                buf, &buf_size);
       if (err >= GNUTLS_E_SUCCESS)
        res = nconc2 (res, list2 (intern (":certificate-id"),
-                                 gnutls_hex_string ((char *)buf,
-                                                    buf_size, "sha1:")));
-      free (buf);
+                                 gnutls_hex_string (buf, buf_size, "sha1:")));
+      xfree (buf);
     }
 
   return res;
 }
 
 DEFUN ("gnutls-peer-status-warning-describe", 
Fgnutls_peer_status_warning_describe, Sgnutls_peer_status_warning_describe, 1, 
1, 0,
-       doc: /* Describe the warning of a GnuTLS peer status from 
`gnutls-peer-status'.*/)
+       doc: /* Describe the warning of a GnuTLS peer status from 
`gnutls-peer-status'.  */)
   (Lisp_Object status_symbol)
 {
   CHECK_SYMBOL (status_symbol);
@@ -1109,9 +1102,9 @@ The return value is a property list with top-level keys 
:warnings and
 }
 
 
-/* Initializes global GnuTLS state to defaults.
-Call `gnutls-global-deinit' when GnuTLS usage is no longer needed.
-Returns zero on success.  */
+/* Initialize global GnuTLS state to defaults.
+   Call `gnutls-global-deinit' when GnuTLS usage is no longer needed.
+   Return zero on success.  */
 static Lisp_Object
 emacs_gnutls_global_init (void)
 {
@@ -1141,8 +1134,8 @@ gnutls_ip_address_p (char *string)
 }
 
 #if 0
-/* Deinitializes global GnuTLS state.
-See also `gnutls-global-init'.  */
+/* Deinitialize global GnuTLS state.
+   See also `gnutls-global-init'.  */
 static Lisp_Object
 emacs_gnutls_global_deinit (void)
 {
@@ -1282,7 +1275,7 @@ one trustfile (usually a CA bundle).  */)
 
   GNUTLS_LOG2 (1, max_log_level, "connecting to host:", c_hostname);
 
-  /* always initialize globals.  */
+  /* Always initialize globals.  */
   global_init = emacs_gnutls_global_init ();
   if (! NILP (Fgnutls_errorp (global_init)))
     return global_init;