[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: Neels Janosch Hofmeyr <neels_at_elego.de>
Date: Sat, 24 May 2008 17:04:57 +0200

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Daniel Shahaf wrote:
> (moving to dev@)
good idea :)

> 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"
Sending x
Transmitting file data .
Committed revision 5.

[trunk] neels_at_dub:~/elego/svn/test/wc/r1796
$ svn log x
- ------------------------------------------------------------------------
r5 | (no author) | 2008-05-24 16:03:27 +0200 (Sat, 24 May 2008) | 1 line

log\n\rmessage\n\r

So, the `\n\r' are taken literally. They are just backslash, then `n'
etc, no CR or LF characters. Does your shell do this differently? How
would I make a -m "message" parameter with CR+LF in it? (like echo -e)

(this case is thus unverified)

=== (2) ===
Use a message file. (use invalid/scrambled line endings)

[trunk] neels_at_dub:~/elego/svn/test/wc/r1796
$ echo -e "log\n\rmessage\n\r" > msgfile

[trunk] neels_at_dub:~/elego/svn/test/wc/r1796
$ cat -A msgfile
log$
^Mmessage$
^M$

[trunk] neels_at_dub:~/elego/svn/test/wc/r1796
$ svn ci -F msgfile
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

=== (3) ===
Use external editor and scrambled line endings.

[trunk] neels_at_dub:~/elego/svn/test/wc/r1796
$ svn ci
subversion/libsvn_client/commit.c:919: (apr_err=135000)
svn: Commit failed (details follow):
subversion/svn/util.c:448: (apr_err=135000)
svn: Error normalizing edited contents to internal format
subversion/libsvn_subr/subst.c:735: (apr_err=135000)
svn: Inconsistent line ending style
subversion/svn/util.c:581: (apr_err=135000)
svn: Your commit message was left in a temporary file:
subversion/svn/util.c:581: (apr_err=135000)
svn: '/arch/elego/svn/test/wc/r1796/svn-commit.2.tmp'

=== (4) ===
There's more still; Above CR/LF sequences are not the usual ones. Line
ending styles in above messages were mixed/scrambled. When I use the
usual (dos) sequence for CR+LF consistently, the commit succeeds:

[trunk] neels_at_dub:~/elego/svn/test/wc/r1796
$ echo -ne "log\r\nmsg\r\n" > msgfile

[trunk] neels_at_dub:~/elego/svn/test/wc/r1796
$ cat -A msgfile
log^M$
msg^M$

[trunk] neels_at_dub:~/elego/svn/test/wc/r1796
$ echo y >> x

[trunk] neels_at_dub:~/elego/svn/test/wc/r1796
$ svn ci -F msgfile
Sending x
Transmitting file data .
Committed revision 10.

[trunk] neels_at_dub:~/elego/svn/test/wc/r1796
$ svn log x | cat -A
- ------------------------------------------------------------------------$
r10 | (no author) | 2008-05-24 16:19:00 +0200 (Sat, 24 May 2008) | 3 lines$
$
log$
msg$
$
...

And the log message is normalized correctly.

=== (5) ===
Try just CR line endings, without LF.

[trunk] neels_at_dub:~/elego/svn/test/wc/r1796
$ echo -ne "log\rmsg\r" > msgfile
[trunk] neels_at_dub:~/elego/svn/test/wc/r1796
$ cat -A msgfile
log^Mmsg^M[trunk] neels_at_dub:~/elego/svn/test/wc/r1796
$ svn ci -F msgfile
Sending x
Transmitting file data .
Committed revision 11.
[trunk] neels_at_dub:~/elego/svn/test/wc/r1796
$ svn log x | cat -A
- ------------------------------------------------------------------------$
r11 | (no author) | 2008-05-24 16:50:09 +0200 (Sat, 24 May 2008) | 3 lines$
$
log$
msg$
$
...

So, this also works.

=====

In summary: the current svn client (trunk) normalizes commit log
messages that are in pure CR+LF or CR format to become LF format.
However, if the line ending styles are mixed or have the wrong order,
the svn client complains about "inconsistent line endings" and aborts
the commit.

>> Can you imagine a nice and easy way of doing this? All I can think of is
>> netcatting recorded transaction data to svnserve... :P
>
> (have you thought of writing a C test?)

Appears that I am ignorant of this option :P
Could you point me at a, preferably simple, example where C is being
used for an existing subversion test?

> Until normalisation is implemented, see above. After normalisation is
> implemented, test the normalisation. :)

done :)

>> ------- Additional comments from neels_at_tigris.org Thu May 22 15:15:58 -0700 2008 -------
>> Is there or may there ever be a revprop that is allowed to have CR ('\r') in its
>> value?
>
> (1) user-defined revprops (which are taken as opaque binary strings)
> (2) Not sure; it might be allowed as 'whitespace' in svn:mime-type and
> friends (unless they're internally normalized as well?)

I am intending to test only "svn:" props for CR characters and reject
any that contain some. Maybe I should just write the patch and run some
tests to see what happens.

- --
Neels Hofmeyr -- elego Software Solutions GmbH
Gustav-Meyer-Allee 25 / Gebäude 12, 13355 Berlin, Germany
phone: +49 30 23458696 mobile: +49 177 2345869 fax: +49 30 23458695
http://www.elegosoft.com | Geschäftsführer: Olaf Wagner | Sitz: Berlin
Handelsreg: Amtsgericht Charlottenburg HRB 77719 | USt-IdNr: DE163214194
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFIOC6ZCXRUkVhi6pARApYqAJ44CpOKmI+WUzCRkSSiv9F5KiA4rQCfX6mG
GS2WUSH3AKeI/Kkfc3sXII8=
=V+Jz
-----END PGP SIGNATURE-----

---------------------------------------------------------------------
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-24 17:05:28 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.