[pkg-wine-party] Bug#579890: grotty: infinite loop when processing a man page

Colin Watson cjwatson at debian.org
Tue May 4 12:26:21 UTC 2010


On Sat, May 01, 2010 at 09:42:04PM -0500, Raphael Geissert wrote:
> grotty appears to enter an infinite loop when processing libwine-dev's 
> (possibly libwine-unstable-dev's too) GetMIMETypeSubKeyA(3w) manpage.
> 
> Reproducing it is as simple as running:
> man --warnings -E UTF-8 -l  GetMIMETypeSubKeyA.3w.gz
> 
> (yep, lintian's check is affected, that's how I noticed it -- not sure if you 
> read the thread on lint-maint.)
> 
> The version on the report is lenny's, but I'm able to reproduce it in sid too 
> (with version 1.20.1-9.)
> I guess that running grotty under the effects of a fuzzer would reveal more 
> bugs.

Thanks.  Here's a reduced test case (run with 'groff -Tutf8 -mandoc'):

  .TH GetMIMETypeSubKeyA 3w "Jun 2009" "Wine API" "Wine API"
  .SH NAME
  \fBGetMIMETypeSubKeyA\fR (SHLWAPI.328)
  .SH NOTES
  .PP
  The base path for the key is \fB"MIME\Database\Content Type\"\fR

There are two bugs here.  The simpler one to fix is the bug in the
manual page (CCed wine at packages.debian.org for this).  It's using \ to
mean a literal backslash, but actually \ introduces a groff escape; \D
emits a drawing command while \C typesets a glyph by name.  This line
should instead read:

  The base path for the key is \fB"MIME\eDatabase\eContent Type\"\fR

The infinite loop in grotty is a bit harder.  What's happening is that
the \D escape emits ditroff commands fairly directly, and in this
particular case you end up with a line that just contains 'Dt', without
the required argument.  In fact, you can reproduce this infinite loop
with just the following grotty input:

  x T utf8
  x res 240 24 40
  x init
  p1
  Dt

The following patch would turn this into a fatal error instead, which
isn't ideal either but is certainly better than an infinite loop.
However, I don't know this code very well and would appreciate review.

2010-05-04  Colin Watson  <cjwatson at debian.org>

	* src/libs/libdriver/input.cpp (get_integer_arg): Emit a fatal error
	on a non-integer argument, bringing the code into line with the
	behaviour documented in the header comment.
	(get_possibly_integer_args): Terminate the loop on a non-integer
	argument.
	(next_arg_begin): Return newline or EOF after emitting the
	corresponding error, rather than continuing on to the next line.

Index: b/src/libs/libdriver/input.cpp
===================================================================
--- a/src/libs/libdriver/input.cpp
+++ b/src/libs/libdriver/input.cpp
@@ -790,7 +790,7 @@
     c = get_char();
   }
   if (!isdigit((int) c))
-    error("integer argument expected");
+    fatal("integer argument expected");
   while (isdigit((int) c)) {
     buf.append(c);
     c = get_char();
@@ -879,6 +879,7 @@
       break;
     default:
       error("integer argument expected");
+      done = true;
       break;
     }
   }
@@ -946,7 +947,7 @@
     case '\n':
     case EOF:
       error("missing argument");
-      break;
+      return c;
     default:			// first essential character
       return c;
     }

Thanks,

-- 
Colin Watson                                       [cjwatson at debian.org]



More information about the pkg-wine-party mailing list