On Tue, May 09, 2023 at 01:36:13PM -0500, Eric Blake wrote: > On Tue, May 09, 2023 at 03:51:21PM +0100, Richard W.M. Jones wrote: > > Debug strings contain all kinds of information including some under > > user control. Previously we simply sent everything to stderr, but > > this is potentially insecure, as well as not dealing well with > > non-printable characters. Escape these strings when printing. > > --- > > server/debug.c | 38 +++++++++++++++++++++++--------------- > > 1 file changed, 23 insertions(+), 15 deletions(-) > > > > diff --git a/server/debug.c b/server/debug.c > > index 177d9d5da..6cf1af316 100644 > > --- a/server/debug.c > > +++ b/server/debug.c > > @@ -42,6 +42,7 @@ > > > > #include "ansi-colours.h" > > #include "open_memstream.h" > > +#include "utils.h" > > > > #include "internal.h" > > > > @@ -76,36 +77,43 @@ debug_common (bool in_server, const char *fs, va_list > > args) > > #ifndef WIN32 > > int tty; > > #endif > > - CLEANUP_FREE char *str = NULL; > > - size_t len = 0; > > - FILE *fp; > > + CLEANUP_FREE char *str_inner = NULL; > > + CLEANUP_FREE char *str_outer = NULL; > > + FILE *fp_inner, *fp_outer; > > + size_t len_inner = 0, len_outer = 0; > > > > - fp = open_memstream (&str, &len); > > - if (fp == NULL) > > + /* The "inner" string is the debug string before escaping. */ > > + fp_inner = open_memstream (&str_inner, &len_inner); > > + if (fp_inner == NULL) > > goto fail; > > + errno = err; > > + vfprintf (fp_inner, fs, args); > > + close_memstream (fp_inner); > > Missing the check for failure. > > > + > > + /* The "outer" string contains the prologue, escaped debug string and > > \n. */ > > + fp_outer = open_memstream (&str_outer, &len_outer); > > + if (fp_outer == NULL) goto fail; > > > > #ifndef WIN32 > > tty = isatty (fileno (stderr)); > > - if (!in_server && tty) ansi_force_colour (ANSI_FG_BOLD_BLACK, fp); > > + if (!in_server && tty) ansi_force_colour (ANSI_FG_BOLD_BLACK, fp_outer); > > #endif > > > > - prologue (fp); > > - > > - errno = err; > > - vfprintf (fp, fs, args); > > + prologue (fp_outer); > > + c_string_quote (str_inner, fp_outer); > > > > #ifndef WIN32 > > - if (!in_server && tty) ansi_force_restore (fp); > > + if (!in_server && tty) ansi_force_restore (fp_outer); > > #endif > > - fprintf (fp, "\n"); > > - if (close_memstream (fp) == -1) > > + fprintf (fp_outer, "\n"); > > + if (close_memstream (fp_outer) == -1) > > Affects multiple patches: close_memstream() (on Linux) fails with EOF, > which happens to be -1 on all sane libc, but which POSIX says can be > any negative value. '< 0' or at least '== EOF' is probably safer than > '== -1'.
I should also change the replacement function to return EOF (also defined as -1 on MinGW). Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW _______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
