Garrett Rooney wrote:
> On 1/11/06, Kamesh Jayachandran <kamesh@collab.net> wrote:
>
>>Hi Julian,
>>Thanks for your feedback.
>>Attaching the new patch.
>>Limiting the header_data to 20 characters as it seems that maximum
>>header name that I see in my dump seems to be 20 characters in length.
>>In case some spurious binary line happen to be there in the stream we
>>limit to print only 20 characters. Don't think it is worth to loop
>>through the characters for the valid printable ascii range. Let me know.
>
> Limiting to 20 characters isn't enough. Putting 20 bytes of arbitrary
> binary crap into an error that'll eventually get printed out to the
> terminal is a bad thing,
It could be annoying, but that's all, I think. The potential for damage is no
greater than trying to locate the erroneous part by running "grep" on the dump
file.
> and can cause security problems (that's why
> apache scrubs data that goes to its log files, it doesn't want to let
> random escape characters through because they can be used to do bad
> things to terminal emulators).
I don't see security as a real concern here. Unlike web server logs which can,
more or less, be triggered by anyone on the net, there is no common scenario in
which someone will unwittingly run "svnadmin load" on a maliciously engineered
dump file. It's normally a rare and manual operation. If someone set up an
server that created repositories from submitted dump files, then stdout/stderr
would probably be captured and not sent straight to a terminal. If a
developer/administrator/helpdesk is experimenting with data from untrusted
sources they should be doing so in a safe environment (e.g. not as 'root').
Little harm can result anyway in most terminals.
> You'll have to verify that the
> characters you're putting in the log are printable, and if not convert
> them to some safe form (numerical encoding of each byte, or
> something).
While that would be polite to the user, I no longer think it is really
necessary. If it is, it should go in our generic error printing code rather
than in this specific case, because there are all sorts of other ways of
getting nasty characters into our error messages - e.g.
$ svn log filename_with_nasty_^[[41m_characters
svn: 'filename_with_nasty__characters' is not under version control
which changes the background to red starting from between the two adjacent
underscores.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Jan 11 20:14:50 2006