Re: dircolors database documentation

[Paul Eggert]

Thanks for catching all these problems.

Eric Blake <address@hidden> writes:

> Oops - we aren't properly quoting ' in dircolors' output.  This should be
> as simple as outputing '\'' in place of ' in append_quoted().

That's a serious bug, no?  It lets an attacker execute arbitrary code.
Admittedly the attack is unlikely, but we should install something
like the following fix right away.  Jim, I assume we're still in a
code-freeze now, so I won't install this, but it does look like a
fairly safe fix....

2005-10-18  Paul Eggert  <address@hidden>

        * src/dircolors.c (append_quoted): Quote ' correctly.
        Problem reported by Eric Blake.

--- dircolors.c.~1.93.~ 2005-09-24 06:40:50.000000000 -0700
+++ dircolors.c 2005-10-18 12:36:17.000000000 -0700
@@ -207,6 +207,13 @@ append_quoted (const char *str)
     {
       switch (*str)
        {
+       case '\'':
+         APPEND_CHAR ('\'');
+         APPEND_CHAR ('\\');
+         APPEND_CHAR ('\'');
+         need_backslash = true;
+         break;
+
        case '\\':
        case '^':
          need_backslash = !need_backslash;



> Oops - we aren't handling NUL.  Before I provide a patch, we need to
> decide if we WANT to support NUL (in which case, we translate NUL into 0
> after unquoted \, into @ after unquoted ^, and into \0 otherwise.

The GNU coding standards say we should support NUL.

> Furthermore, this means it is possible to generate suffix rules that will
> never match a filename (since the basename of a file cannot contain / or
> embedded NUL).

It's simplest to not match in that case, without reporting an error.
Isn't that what the code does already?  If so, we're done.

> I think the best thing here it to treat the unterminated escape as
> an error.

Yes, that sounds right to me as well.