[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [Issue 1796] defective or malicious client can corrupt repository log messages

From: Daniel Shahaf <d.s_at_daniel.shahaf.co.il>
Date: Sun, 25 May 2008 08:54:45 +0300 (Jerusalem Daylight Time)

Neels Janosch Hofmeyr wrote on Sun, 25 May 2008 at 01:19 +0200:
> Daniel Shahaf wrote:
> > Neels Janosch Hofmeyr wrote on Sat, 24 May 2008 at 17:04 +0200:
> >> Daniel Shahaf wrote:
> >>> neels_at_tigris.org wrote on Fri, 22 May 2008 at 22:04 -0000:
> >>>> http://subversion.tigris.org/issues/show_bug.cgi?id=1796
> >>>>
> >>> According to my tests, 'svn' doesn't normalise log messages to CRLF now
> >>> (though it would be nice if it did), so 'svn ci -m "foo\r\nbar"' or svn
> >>> ci -F should do.
> >> Well, according to my tests ;), it does. Let's break down the different
> >> cases, using r31375:
> >>
> >> === (1) ===
> >> Try and pass commandline message.
> >>
> >> [trunk] neels_at_dub:~/elego/svn/test/wc/r1796
> >> $ svn ci -m "log\n\rmessage\n\r"
> > ^^^^
> > That's LFCR, not CRLF. I suppose you did that intentionally?
> Also yes.
>
> [...]
>
> >> How would I make a -m "message" parameter with CR+LF in it? (like echo
> >> -e)
> >>
> >
> > -m "`printf`" ? -m "`cat CRLF-file`" ? python -c 'system()' ? Or throw
> > it in a run_svn() call and see if it works :)
> >
> >> (this case is thus unverified)
>
> Got it covered by this:
>
> $ echo -en "inconsistent\rlog\n\rmsg\r\n" > msg
> $ cat -A msg
> inconsistent^Mlog$
> ^Mmsg^M$
>
> $ echo y >> x
> $ svn ci -m "`cat msg`"
> subversion/libsvn_client/commit.c:919: (apr_err=135000)
> svn: Commit failed (details follow):
> subversion/svn/util.c:660: (apr_err=135000)
> svn: Error normalizing log message to internal format
> subversion/libsvn_subr/subst.c:735: (apr_err=135000)
> svn: Inconsistent line ending style
>
> So, the client also normalizes -m messages.
>
> But, observe:
>
> $ echo -en "log\rmsg\n" > msg
>
> $ cat -A msg
> log^Mmsg$
>
> $ echo y >> x
>
> $ svn ci -F msg
> subversion/libsvn_client/commit.c:919: (apr_err=135000)
> svn: Commit failed (details follow):
> subversion/svn/util.c:660: (apr_err=135000)
> svn: Error normalizing log message to internal format
> subversion/libsvn_subr/subst.c:735: (apr_err=135000)
> svn: Inconsistent line ending style
>
> $ svn ci -m "`cat msg`"
> Sending x
> Transmitting file data .
> Committed revision 5.
>
> $ svn log x | cat -A
> -------------------------------------------------------------------------$
> r5 | (no author) | 2008-05-25 01:03:01 +0200 (Sun, 25 May 2008) | 2 lines$
> $
> log$
> msg$
> -------------------------------------------------------------------------$
> r4 | (no author) | 2008-05-25 00:55:37 +0200 (Sun, 25 May 2008) | 2 lines$
> [...]
>
>
> Why, I am putting inconsistent line endings on the -m command line, and
> the same data that ci -F file rejects is accepted by ci -m "`cat file`".

As you say below, the cmdline client removes trailing newlines from its
-m argument's value. In this case the value is foo\rbar\n, which after
removing the last \n becomes consistent (contains \r's only) and thus
allowed. In other words, this observation is consistent with the theory
you introduce below.

> Furthermore:
>
> $ echo -en "log\rmsg\n\n" > msg
> $ cat -A msg
> log^Mmsg$
> $
> $ echo y >> x
> $ svn ci -m "`cat msg`"
> Sending x
> Transmitting file data .
> Committed revision 6.
> [trunk] neels_at_dub:/arch/elego/svn/test/wc/r1796
> $ svn log x | cat -A
> -------------------------------------------------------------------------$
> r6 | (no author) | 2008-05-25 01:04:19 +0200 (Sun, 25 May 2008) | 2 lines$
> $
> log$
> msg$
> -------------------------------------------------------------------------$
> r5 | (no author) | 2008-05-25 01:03:01 +0200 (Sun, 25 May 2008) | 2 lines$
> $
> log$
> msg$
> -------------------------------------------------------------------------$
> r4 | (no author) | 2008-05-25 00:55:37 +0200 (Sun, 25 May 2008) | 2 lines$
> $
> [...]
>
> Adding another newline to the commit message does not add a further
> newline in the repository. In the process of evaluating the -m argument,
> it is probably stripped of trailing whitespace. I guess that's good
> enough, even though it's confusing at first glance. (Note that trailing
> newlines in a *valid* log message file are not stripped)
>
> ...in summary, the svn commandline client behaves well at normalizing
> commit log messages' line ending style, received whichever way.
>
> Thanks for your help, Daniel!
>

Thank you for investigating this point so thoroughly. :) I was going to
ask how you meant to proceed and if you were waiting for review of the
patch you attached to the issue, but I see you're discussing this with
Karl on another thread.

Good luck,

Daniel

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-05-25 07:55:20 CEST

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.