Two Application Hangs in ‘gnupg’ Resulting from Failure to Check File Type

[Preston Moore]

*** I’m re-sending this as I never saw it appear on the list and it doesn’t appear in the archive.  I guess my registration wasn’t active at the time.  Apologies if this is a duplicate. ***

Hey everyone,

There are two situations where gnupg can be made to hang during execution.  In the first case, a denial of service situation can occur when pubring.gpg or secring.gpg are replaced with a FIFO as the application ends up blocking forever waiting for data. In the second case,  because the type of file containing a key to be imported with is not checked, gnupg can be made to spin forever attempting to process what amounts to an infinitely large file.  Both of these situations can be particularly damaging when if gnupg is being run as part of an unattended process, such as a cron job, where the hang may go unnoticed.  An unnoticed hang caused by the second situation can affect system performance as gnupg spins consuming as much processing capacity as it can.

These problems were found as part of an effort to detect and deal with “environmental” bugs in popular applications (for more information, check out https://works-everywhere.org). They were found using a tool that detects situations where an application fails to correctly handle unusual environmental conditions such as files having an unexpected file type.

The following patch confirms that the file containing a key resource is regular file before processing its contents:

---
g10/keydb.c | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/g10/keydb.c b/g10/keydb.c
index 0f28bc3..1b5e095 100644
--- a/g10/keydb.c
+++ b/g10/keydb.c
@@ -650,6 +650,7 @@ keydb_add_resource (const char *url, unsigned int flags)
  gpg_error_t err = 0;
  KeydbResourceType rt = KEYDB_RESOURCE_TYPE_NONE;
  void *token;
+  struct stat sb;

  /* Create the resource if it is the first registered one.  */
  create = (!read_only && !any_registered);
@@ -692,6 +693,13 @@ keydb_add_resource (const char *url, unsigned int flags)
  else
    filename = xstrdup (resname);

+  if(stat(filename, &sb) == -1 || !S_ISREG(sb.st_mode))
+    {
+       log_error ("key resource is not a regular file");
+       err = gpg_error (GPG_ERR_INV_KEYRING);
+       goto leave;
+    }
+
  /* See whether we can determine the filetype.  */
  if (rt == KEYDB_RESOURCE_TYPE_NONE)
    {
--
2.7.4


The below patch to checks the file type of the file containing a key being imported and rejects it if the file is not a regular file:


---
g10/import.c | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/g10/import.c b/g10/import.c
index d9d658b..1c9a861 100644
--- a/g10/import.c
+++ b/g10/import.c
@@ -23,6 +23,7 @@
#include <stdlib.h>
#include <string.h>
#include <errno.h>
+#include <sys/stat.h>

#include "gpg.h"
#include "options.h"
@@ -431,6 +432,7 @@ import_keys_internal (ctrl_t ctrl, iobuf_t inp, char **fnames, int nnames,
  int i;
  int rc = 0;
  struct import_stats_s *stats = stats_handle;
+  struct stat sb;

  if (!stats)
    stats = import_new_stats_handle ();
@@ -448,6 +450,12 @@ import_keys_internal (ctrl_t ctrl, iobuf_t inp, char **fnames, int nnames,
      for (i=0; i < nnames; i++)
        {
          const char *fname = fnames? fnames[i] : NULL;
+          if(!(stat(fname, &sb) == 0) || !S_ISREG(sb.st_mode))
+            {
+                gpg_err_set_errno(EINVAL);
+                log_error ("%s is an invalid file type\n", fname);
+                break;
+            }
          IOBUF inp2 = iobuf_open(fname);

          if (!fname)
--
2.7.4


Thanks,
Preston Moore