[PATCH GnuPG] Fix handling of I/O errors
Demi Marie Obenour
demiobenour at gmail.com
Thu Feb 2 07:16:19 CET 2023
Errors writing to stdout, stderr, or the status file must result in a
non-zero status code. Also BUG() if a CR or LF would be injected into
the status stream, escape bytes above 126, add a log_assert(), check
the return value of snprintf(), and escape backslashes in
write_status_printf(). Finally, ensure that errors writing to
the status file cause all status output to stop immediately.
GnuPG-bug-id: T6185
Signed-off-by: Demi Marie Obenour <demiobenour at gmail.com>
---
g10/cpr.c | 188 ++++++++++++++++++++++++++++++++++++-----------------
g10/gpg.c | 3 +
g10/gpgv.c | 2 +
g10/main.h | 1 +
4 files changed, 134 insertions(+), 60 deletions(-)
diff --git a/g10/cpr.c b/g10/cpr.c
index 79eff314375f07380f7752736ddf2613dbe8b378..f60c745bf98a6536aa048f2685ef3341664b4bb8 100644
--- a/g10/cpr.c
+++ b/g10/cpr.c
@@ -49,14 +49,17 @@ progress_cb (void *ctx, const char *what, int printchar,
int current, int total)
{
char buf[50];
+ int res;
(void)ctx;
if ( printchar == '\n' && !strcmp (what, "primegen") )
- snprintf (buf, sizeof buf, "%.20s X 100 100", what );
+ res = snprintf (buf, sizeof buf, "%.20s X 100 100", what );
else
- snprintf (buf, sizeof buf, "%.20s %c %d %d",
- what, printchar=='\n'?'X':printchar, current, total );
+ res = snprintf (buf, sizeof buf, "%.20s %c %d %d",
+ what, printchar=='\n'?'X':printchar, current, total );
+ if ((unsigned)res >= sizeof buf)
+ abort();
write_status_text (STATUS_PROGRESS, buf);
}
@@ -140,24 +143,41 @@ write_status ( int no )
write_status_text( no, NULL );
}
+int
+check_status_error (void)
+{
+ if (!statusfp)
+ return 0;
+ if (!es_fflush (statusfp) && !es_ferror(statusfp))
+ return 0;
+ if (opt.exit_on_status_write_error)
+ g10_exit (1);
+ if (statusfp != es_stdout && statusfp != es_stderr)
+ es_fclose(statusfp);
+ statusfp = NULL;
+ return 1;
+}
/* Write a status line with code NO followed by the string TEXT and
* directly followed by the remaining strings up to a NULL. Embedded
- * CR and LFs in the strings (but not in TEXT) are C-style escaped.*/
+ * CR and LFs in the strings and TEXT are C-style escaped.*/
void
write_status_strings (int no, const char *text, ...)
{
va_list arg_ptr;
const char *s;
+ int err = 0;
if (!statusfp || !status_currently_allowed (no) )
return; /* Not enabled or allowed. */
- es_fputs ("[GNUPG:] ", statusfp);
- es_fputs (get_status_string (no), statusfp);
+ if (es_fputs ("[GNUPG:] ", statusfp) ||
+ es_fputs (get_status_string (no), statusfp))
+ goto bad;
if ( text )
{
- es_putc ( ' ', statusfp);
+ if ( es_putc ( ' ', statusfp) != ' ' )
+ goto bad;
va_start (arg_ptr, text);
s = text;
do
@@ -165,19 +185,24 @@ write_status_strings (int no, const char *text, ...)
for (; *s; s++)
{
if (*s == '\n')
- es_fputs ("\\n", statusfp);
+ err = es_fputs ("\\n", statusfp);
else if (*s == '\r')
- es_fputs ("\\r", statusfp);
+ err = es_fputs ("\\r", statusfp);
+ else if (*s == '\\')
+ err = es_fputs ("\\\\", statusfp);
else
- es_fputc (*(const byte *)s, statusfp);
+ err = es_fputc (*(const byte *)s, statusfp);
+ if (err == EOF)
+ goto bad;
}
}
while ((s = va_arg (arg_ptr, const char*)));
va_end (arg_ptr);
}
es_putc ('\n', statusfp);
- if (es_fflush (statusfp) && opt.exit_on_status_write_error)
- g10_exit (0);
+
+bad:
+ check_status_error();
}
@@ -190,6 +215,7 @@ write_status_strings2 (ctrl_t dummy, int no, ...)
{
va_list arg_ptr;
const char *s;
+ int err = 0;
(void)dummy;
@@ -198,29 +224,33 @@ write_status_strings2 (ctrl_t dummy, int no, ...)
va_start (arg_ptr, no);
- es_fputs ("[GNUPG:] ", statusfp);
- es_fputs (get_status_string (no), statusfp);
+ if (es_fputs ("[GNUPG:] ", statusfp) ||
+ es_fputs (get_status_string (no), statusfp))
+ goto bad;
while ((s = va_arg (arg_ptr, const char*)))
{
- if (*s)
- es_putc (' ', statusfp);
+ if (*s && es_putc (' ', statusfp) != ' ')
+ goto bad;
for (; *s; s++)
{
if (*s == '\n')
- es_fputs ("\\n", statusfp);
+ err = es_fputs ("\\n", statusfp);
else if (*s == '\r')
- es_fputs ("\\r", statusfp);
+ err = es_fputs ("\\r", statusfp);
+ else if (*s == '\\')
+ err = es_fputs ("\\\\", statusfp);
else
- es_fputc (*(const byte *)s, statusfp);
+ err = es_fputc (*(const byte *)s, statusfp);
+ if (err == EOF)
+ goto bad;
}
}
es_putc ('\n', statusfp);
va_end (arg_ptr);
- if (es_fflush (statusfp) && opt.exit_on_status_write_error)
- g10_exit (0);
-
+bad:
+ check_status_error();
return 0;
}
@@ -239,45 +269,52 @@ write_status_printf (int no, const char *format, ...)
{
va_list arg_ptr;
char *buf;
+ int err = 0;
if (!statusfp || !status_currently_allowed (no) )
return; /* Not enabled or allowed. */
- es_fputs ("[GNUPG:] ", statusfp);
- es_fputs (get_status_string (no), statusfp);
+ va_start (arg_ptr, format);
+ if (es_fputs ("[GNUPG:] ", statusfp) ||
+ es_fputs (get_status_string (no), statusfp))
+ goto bad;
if (format)
{
- es_putc ( ' ', statusfp);
- va_start (arg_ptr, format);
+ if (es_fputc ( ' ', statusfp ) != ' ')
+ goto bad;
buf = gpgrt_vbsprintf (format, arg_ptr);
if (!buf)
- log_error ("error printing status line: %s\n",
- gpg_strerror (gpg_err_code_from_syserror ()));
+ {
+ log_error ("error printing status line: %s\n",
+ gpg_strerror (gpg_err_code_from_syserror ()));
+ goto bad;
+ }
else
{
- if (strpbrk (buf, "\r\n"))
+ if (strpbrk (buf, "\r\n\\"))
{
const byte *s;
- for (s=buf; *s; s++)
+ for (s=buf; *s && err != EOF; s++)
{
if (*s == '\n')
- es_fputs ("\\n", statusfp);
+ err = es_fputs ("\\n", statusfp);
else if (*s == '\r')
- es_fputs ("\\r", statusfp);
+ err = es_fputs ("\\r", statusfp);
+ else if (*s == '\\')
+ err = es_fputs ("\\\\", statusfp);
else
- es_fputc (*s, statusfp);
+ err = es_fputc (*s, statusfp);
}
}
- else
- es_fputs (buf, statusfp);
+ else if (es_fputs (buf, statusfp))
+ goto bad;
gpgrt_free (buf);
}
-
- va_end (arg_ptr);
}
es_putc ('\n', statusfp);
- if (es_fflush (statusfp) && opt.exit_on_status_write_error)
- g10_exit (0);
+bad:
+ check_status_error();
+ va_end (arg_ptr);
}
@@ -288,10 +325,12 @@ write_status_error (const char *where, gpg_error_t err)
if (!statusfp || !status_currently_allowed (STATUS_ERROR))
return; /* Not enabled or allowed. */
+ if (!where || strpbrk(where, "\r\n"))
+ BUG();
+
es_fprintf (statusfp, "[GNUPG:] %s %s %u\n",
get_status_string (STATUS_ERROR), where, err);
- if (es_fflush (statusfp) && opt.exit_on_status_write_error)
- g10_exit (0);
+ check_status_error();
}
@@ -302,10 +341,12 @@ write_status_errcode (const char *where, int errcode)
if (!statusfp || !status_currently_allowed (STATUS_ERROR))
return; /* Not enabled or allowed. */
+ if (!where || strpbrk(where, "\r\n"))
+ BUG();
+
es_fprintf (statusfp, "[GNUPG:] %s %s %u\n",
get_status_string (STATUS_ERROR), where, gpg_err_code (errcode));
- if (es_fflush (statusfp) && opt.exit_on_status_write_error)
- g10_exit (0);
+ check_status_error();
}
@@ -315,6 +356,9 @@ write_status_failure (const char *where, gpg_error_t err)
{
static int any_failure_printed;
+ if (!where || strpbrk(where, "\r\n"))
+ BUG();
+
if (!statusfp || !status_currently_allowed (STATUS_FAILURE))
return; /* Not enabled or allowed. */
if (any_failure_printed)
@@ -322,8 +366,7 @@ write_status_failure (const char *where, gpg_error_t err)
any_failure_printed = 1;
es_fprintf (statusfp, "[GNUPG:] %s %s %u\n",
get_status_string (STATUS_FAILURE), where, err);
- if (es_fflush (statusfp) && opt.exit_on_status_write_error)
- g10_exit (0);
+ check_status_error();
}
@@ -342,14 +385,19 @@ write_status_text_and_buffer (int no, const char *string,
int lower_limit = ' ';
size_t n, count, dowrap;
- if (!statusfp || !status_currently_allowed (no))
- return; /* Not enabled or allowed. */
+ if (string && strpbrk(string, "\r\n"))
+ BUG();
if (wrap == -1)
{
lower_limit--;
wrap = 0;
}
+ else if (wrap < 0)
+ BUG();
+
+ if (!statusfp || !status_currently_allowed (no) )
+ return; /* Not enabled or allowed. */
text = get_status_string (no);
count = dowrap = first = 1;
@@ -357,16 +405,20 @@ write_status_text_and_buffer (int no, const char *string,
{
if (dowrap)
{
- es_fprintf (statusfp, "[GNUPG:] %s ", text);
+ if (es_fprintf (statusfp, "[GNUPG:] %s ", text) != strlen(text) + 10)
+ goto bad;
count = dowrap = 0;
if (first && string)
{
- es_fputs (string, statusfp);
- count += strlen (string);
+ size_t size = strlen (string);
+ if (es_fwrite (string, 1, size, statusfp) != size)
+ goto bad;
+ count += size;
/* Make sure that there is a space after the string. */
- if (*string && string[strlen (string)-1] != ' ')
+ if (*string && string[size-1] != ' ')
{
- es_putc (' ', statusfp);
+ if (es_putc (' ', statusfp) != ' ')
+ goto bad;
count++;
}
}
@@ -375,33 +427,38 @@ write_status_text_and_buffer (int no, const char *string,
for (esc=0, s=buffer, n=len; n; s++, n--)
{
if (*s == '%' || *(const byte*)s <= lower_limit
- || *(const byte*)s == 127 )
+ || *(const byte*)s >= 127 )
esc = 1;
if (wrap && ++count > wrap)
dowrap=1;
if (esc || dowrap)
break;
}
- if (s != buffer)
- es_fwrite (buffer, s-buffer, 1, statusfp);
+ log_assert(s >= buffer);
+ if (s != buffer && es_fwrite (buffer, 1, s-buffer, statusfp) != s-buffer)
+ goto bad;
if ( esc )
{
- es_fprintf (statusfp, "%%%02X", *(const byte*)s );
+ if (es_fprintf (statusfp, "%%%02X", *(const byte*)s ) != 4)
+ goto bad;
s++; n--;
}
buffer = s;
len = n;
if (dowrap && len)
- es_putc ('\n', statusfp);
+ {
+ es_putc ('\n', statusfp);
+ if (check_status_error())
+ return;
+ }
}
while (len);
es_putc ('\n',statusfp);
- if (es_fflush (statusfp) && opt.exit_on_status_write_error)
- g10_exit (0);
+bad:
+ check_status_error();
}
-
void
write_status_buffer (int no, const char *buffer, size_t len, int wrap)
{
@@ -703,3 +760,14 @@ cpr_get_answer_okay_cancel (const char *keyword,
}
}
}
+
+int
+check_io_error (void)
+{
+ return 0;
+ opt.exit_on_status_write_error = 0;
+ if (es_fflush(es_stdout) || es_fflush(es_stderr) ||
+ check_status_error() || es_ferror(es_stdin) ||
+ es_ferror(es_stdout) || es_ferror(es_stderr))
+ log_error("I/O error");
+}
diff --git a/g10/gpg.c b/g10/gpg.c
index b9a81510fc967eac557a9fceefbdcef481c11f3b..1982b39cc85a91dc867514b393bc0eb5139a4d09 100644
--- a/g10/gpg.c
+++ b/g10/gpg.c
@@ -5365,6 +5365,9 @@ main (int argc, char **argv)
release_armor_context (afx);
FREE_STRLIST(remusr);
FREE_STRLIST(locusr);
+
+ check_io_error();
+
g10_exit(0);
return 8; /*NEVER REACHED*/
}
diff --git a/g10/gpgv.c b/g10/gpgv.c
index ceded4af9e62f59acd970feb2d349aa47a4c352d..9d4f7b55ef07b05d2334afc987ed63848b83424a 100644
--- a/g10/gpgv.c
+++ b/g10/gpgv.c
@@ -281,6 +281,8 @@ main( int argc, char **argv )
keydb_release (ctrl->cached_getkey_kdb);
xfree (ctrl);
+ check_io_error();
+
/* cleanup */
g10_exit (0);
return 8; /*NOTREACHED*/
diff --git a/g10/main.h b/g10/main.h
index 62d2651be502a2fec710ac682fe5fc77cb47cf99..c08a839ff7c131a4e4e05e421390d4381c20bc65 100644
--- a/g10/main.h
+++ b/g10/main.h
@@ -216,6 +216,7 @@ void write_status_text_and_buffer ( int no, const char *text,
const char *buffer, size_t len, int wrap );
void write_status_begin_signing (gcry_md_hd_t md);
+int check_io_error (void);
int cpr_enabled(void);
--
2.39.1
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_0xB288B55FFF9C22C1.asc
Type: application/pgp-keys
Size: 4885 bytes
Desc: OpenPGP public key
URL: <https://lists.gnupg.org/pipermail/gnupg-devel/attachments/20230202/ef68ffea/attachment-0001.key>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.gnupg.org/pipermail/gnupg-devel/attachments/20230202/ef68ffea/attachment-0001.sig>
More information about the Gnupg-devel
mailing list