Re: [PATCH] t/import: test for nasty characters

[Eric Wong <e@yhbt.net>]

"Eric W. Biederman" <ebiederm@xmission.com> wrote:
> Eric Wong <e@yhbt.net> writes:
> 
> > Eric Wong <e@yhbt.net> wrote:
> >> "Eric W. Biederman" <ebiederm@xmission.com> wrote:
> >> > -		$name =~ tr/<>//d;
> >> > +		$name =~ tr/\n\r<>$/     /d;
> >> 
> >> Is getting rid of '$' an effort to avoid double interpolation by Perl?
> >> Perl won't recursively expand variables AFAIK.
> >
> > I'm not seeing the purpose in $ being grouped with the
> > characters (test below confirms it, I think).
> 
> What I think we should be doing is any characters that are not a valid
> part of a name (as defined by the appropriate email RFCs) should be
> dealt with.
> 
> I am pretty certain $ isn't of those characters that is valid in a name.

*shrug*  We'd have to dig through RFCs for that, but practically
anything can be valid once base64-encoded in headers.

This part of the code is only about keeping git-fast-import
happy.  '$' is a visible character, and can be used creatively
for "fun" usernames, so I prefer we keep it.

Fwiw, I was able to send an email to a big mail provider w/o
getting flagged as spam using "Ca$h Wong" as my From: name.
mutt didn't even escape or quote it, either.

> Otherwise I suspect this will be a game of whack-a-mole as new weird
> and strange cases crop up.  Maybe I am just paranoid, but right now
> the code seems a bit too liberal in what it accepts.

I prefer we keep Import.pm liberal.  MDA and spam filters can be
stricter, of course.

I can't see '$' possibly doing any damage unless somebody runs
`eval' on From: headers; but if we're defending against that,
we'd have to disallow words like "rm" and "unlink", too :)