From 0bb605baf8702d1a5d1ee16f52b07840d88616e7 Mon Sep 17 00:00:00 2001 From: joey Date: Sun, 2 Jul 2006 02:18:31 +0000 Subject: [PATCH] * Parse svn log as xml for improved utf8 and security. Note that this makes ikiwiki depend on XML::Simple. Patch by Faidon Liambotis. --- IkiWiki/CGI.pm | 8 ++- IkiWiki/Rcs/svn.pm | 112 ++++++++++++++++++------------------- debian/changelog | 4 +- debian/control | 2 +- doc/bugs/utf8_svn_log.mdwn | 2 + doc/install.mdwn | 3 +- doc/security.mdwn | 23 +++++--- 7 files changed, 84 insertions(+), 70 deletions(-) diff --git a/IkiWiki/CGI.pm b/IkiWiki/CGI.pm index 30af53586..f589ca41d 100644 --- a/IkiWiki/CGI.pm +++ b/IkiWiki/CGI.pm @@ -33,6 +33,11 @@ sub cgi_recentchanges ($) { #{{{ unlockwiki(); + # Force reading the template as utf-8, necessary if + # rcs_recentchanges returns true utf-8 strings. + open(TMPL, "<:utf8", "$config{templatedir}/recentchanges.tmpl"); + my $template=HTML::Template->new(filehandle => *TMPL); + close(TMPL); my $template=HTML::Template->new( filename => "$config{templatedir}/recentchanges.tmpl" ); @@ -44,8 +49,7 @@ sub cgi_recentchanges ($) { #{{{ styleurl => styleurl(), baseurl => "$config{url}/", ); - require Encode; - print $q->header(-charset=>'utf-8'), Encode::decode_utf8($template->output); + print $q->header(-charset=>'utf-8'), $template->output; } #}}} sub cgi_signin ($$) { #{{{ diff --git a/IkiWiki/Rcs/svn.pm b/IkiWiki/Rcs/svn.pm index 61c6409a7..5474e3be2 100644 --- a/IkiWiki/Rcs/svn.pm +++ b/IkiWiki/Rcs/svn.pm @@ -7,7 +7,6 @@ use IkiWiki; package IkiWiki; -my $svn_log_infoline=qr/^r(\d+)\s+\|\s+([^\s]+)\s+\|\s+(\d+-\d+-\d+\s+\d+:\d+:\d+\s+[-+]?\d+).*/; my $svn_webcommit=qr/^web commit by (\w+):?(.*)/; sub svn_info ($$) { #{{{ @@ -99,67 +98,64 @@ sub rcs_recentchanges ($) { #{{{ my $num=shift; my @ret; + return unless -d "$config{srcdir}/.svn"; + eval q{use CGI 'escapeHTML'}; eval q{use Date::Parse}; eval q{use Time::Duration}; + eval q{use XML::Simple}; - if (-d "$config{srcdir}/.svn") { - my $svn_url=svn_info("URL", $config{srcdir}); + my $svn_url=svn_info("URL", $config{srcdir}); + my $xml = XMLin(scalar `svn --xml -v log '$svn_url'`, + ForceArray => [ 'logentry', 'path' ], + GroupTags => { paths => 'path' }, + KeyAttr => { path => 'content' }, + ); + foreach my $logentry (@{$xml->{logentry}}) { + my (@pages, @message); + + my $rev = $logentry->{revision}; + my $user = $logentry->{author}; + + my $date = $logentry->{date}; + $date =~ s/T/ /; + $date =~ s/\.\d+Z$//; + my $when=concise(ago(time - str2time($date, 'UTC'))); + + foreach my $msgline (split(/\n/, $logentry->{msg})) { + push @message, { line => escapeHTML($msgline) }; + } - my $div=qr/^--------------------+$/; - my $state='start'; - my ($rev, $user, $when, @pages, @message); - foreach (`LANG=C svn log -v '$svn_url'`) { - chomp; - if ($state eq 'start' && /$div/) { - $state='header'; - } - elsif ($state eq 'header' && /$svn_log_infoline/) { - $rev=$1; - $user=$2; - $when=concise(ago(time - str2time($3))); - } - elsif ($state eq 'header' && /^\s+[A-Z]+\s+\/\Q$config{svnpath}\E\/([^ ]+)(?:$|\s)/) { - my $file=$1; - my $diffurl=$config{diffurl}; - $diffurl=~s/\[\[file\]\]/$file/g; - $diffurl=~s/\[\[r1\]\]/$rev - 1/eg; - $diffurl=~s/\[\[r2\]\]/$rev/g; - push @pages, { - link => htmllink("", "", pagename($file), 1), - diffurl => $diffurl, - } if length $file; - } - elsif ($state eq 'header' && /^$/) { - $state='body'; - } - elsif ($state eq 'body' && /$div/) { - my $committype="web"; - if (defined $message[0] && - $message[0]->{line}=~/$svn_webcommit/) { - $user="$1"; - $message[0]->{line}=$2; - } - else { - $committype="svn"; - } - - push @ret, { rev => $rev, - user => htmllink("", "", $user, 1), - committype => $committype, - when => $when, message => [@message], - pages => [@pages], - } if @pages; - return @ret if @ret >= $num; - - $state='header'; - $rev=$user=$when=undef; - @pages=@message=(); - } - elsif ($state eq 'body') { - push @message, {line => escapeHTML($_)}, - } + my $committype="web"; + if (defined $message[0] && + $message[0]->{line}=~/$svn_webcommit/) { + $user="$1"; + $message[0]->{line}=$2; + } + else { + $committype="svn"; } + + foreach (keys %{$logentry->{paths}}) { + next unless /^\/\Q$config{svnpath}\E\/([^ ]+)(?:$|\s)/; + my $file=$1; + my $diffurl=$config{diffurl}; + $diffurl=~s/\[\[file\]\]/$file/g; + $diffurl=~s/\[\[r1\]\]/$rev - 1/eg; + $diffurl=~s/\[\[r2\]\]/$rev/g; + push @pages, { + link => htmllink("", "", pagename($file), 1), + diffurl => $diffurl, + } if length $file; + } + push @ret, { rev => $rev, + user => htmllink("", "", $user, 1), + committype => $committype, + when => $when, + message => [@message], + pages => [@pages], + } if @pages; + return @ret if @ret >= $num; } return @ret; @@ -230,6 +226,8 @@ sub rcs_notify () { #{{{ sub rcs_getctime ($) { #{{{ my $file=shift; eval q{use Date::Parse}; + + my $svn_log_infoline=qr/^r\d+\s+\|\s+[^\s]+\s+\|\s+(\d+-\d+-\d+\s+\d+:\d+:\d+\s+[-+]?\d+).*/; my $child = open(SVNLOG, "-|"); if (! $child) { @@ -239,7 +237,7 @@ sub rcs_getctime ($) { #{{{ my $date; while () { if (/$svn_log_infoline/) { - $date=$3; + $date=$1; } } close SVNLOG || warn "svn log $file exited $?"; diff --git a/debian/changelog b/debian/changelog index 50f9c1e60..0b0a61fd0 100644 --- a/debian/changelog +++ b/debian/changelog @@ -6,8 +6,10 @@ ikiwiki (1.8) UNRELEASED; urgency=low about changes to rss feeds. * Honor LC_CTIME when formatting a time for display. Thanks, Faidon Liambotis. + * Parse svn log as xml for improved utf8 and security. Note that this makes + ikiwiki depend on XML::Simple. Patch by Faidon Liambotis. - -- Joey Hess Sat, 1 Jul 2006 20:28:26 -0400 + -- Joey Hess Sat, 1 Jul 2006 20:41:55 -0400 ikiwiki (1.7) unstable; urgency=low diff --git a/debian/control b/debian/control index 364c80aed..168c57e3d 100644 --- a/debian/control +++ b/debian/control @@ -8,7 +8,7 @@ Standards-Version: 3.7.2 Package: ikiwiki Architecture: all -Depends: ${perl:Depends}, markdown, libtimedate-perl, libhtml-template-perl, libhtml-scrubber-perl, libcgi-formbuilder-perl (>= 3.02.02), libtime-duration-perl, libcgi-session-perl, libmail-sendmail-perl, gcc | c-compiler, libc6-dev | libc-dev +Depends: ${perl:Depends}, libxml-simple-perl, markdown, libtimedate-perl, libhtml-template-perl, libhtml-scrubber-perl, libcgi-formbuilder-perl (>= 3.02.02), libtime-duration-perl, libcgi-session-perl, libmail-sendmail-perl, gcc | c-compiler, libc6-dev | libc-dev Recommends: subversion | git-core, hyperestraier Suggests: viewcvs, librpc-xml-perl Description: a wiki compiler diff --git a/doc/bugs/utf8_svn_log.mdwn b/doc/bugs/utf8_svn_log.mdwn index 7266ab926..abd957719 100644 --- a/doc/bugs/utf8_svn_log.mdwn +++ b/doc/bugs/utf8_svn_log.mdwn @@ -7,3 +7,5 @@ have that locale. Seems that the right fix for this is to use svn log --xml, which is always utf-8 and come up with a parser for that. Also fixes the spoofing issue in [[security]]. + +[[bugs/done]] diff --git a/doc/install.mdwn b/doc/install.mdwn index 0aa55fb0b..f65e1a227 100644 --- a/doc/install.mdwn +++ b/doc/install.mdwn @@ -3,7 +3,8 @@ The easiest way to install ikiwiki is using the Debian package. Ikiwiki requires [[MarkDown]] be installed, and also uses the following perl modules if available: `CGI::Session` `CGI::FormBuilder` (version 3.02.02 or newer) `HTML::Template` `Mail::SendMail` `Time::Duration` -`Date::Parse` (libtimedate-perl), `HTML::Scrubber`, `RPC::XML` +`Date::Parse` (libtimedate-perl), `HTML::Scrubber`, `RPC::XML`, +`XML::Simple` If you want to install from the tarball, you should make sure that the required perl modules are installed, then run: diff --git a/doc/security.mdwn b/doc/security.mdwn index 53000c08e..b294decc8 100644 --- a/doc/security.mdwn +++ b/doc/security.mdwn @@ -12,17 +12,16 @@ to be kept in mind. _(The list of things to fix.)_ -## svn commit logs +## commit spoofing -Anyone with svn commit access can forge "web commit from foo" and make it -appear on [[RecentChanges]] like foo committed. One way to avoid this would -be to limit web commits to those done by a certian user. +Anyone with direct commit access can forge "web commit from foo" and +make it appear on [[RecentChanges]] like foo committed. One way to avoid +this would be to limit web commits to those done by a certian user. -It's actually possible to force a whole series of svn commits to appear to -have come just before yours, by forging svn log output. This could be -guarded against by using svn log --xml. +## other stuff to look at -ikiwiki escapes any html in svn commit logs to prevent other mischief. +I need to audit the git backend a bit, and have been meaning to +see if any CRLF injection type things can be done. ---- @@ -227,3 +226,11 @@ only render a file with that extension. ikiwiki supports protecting users from their own broken browsers via the [[plugins/htmlscrubber]] plugin, which is enabled by default. + +## svn commit logs + +It's was possible to force a whole series of svn commits to appear to +have come just before yours, by forging svn log output. This was +guarded against by using svn log --xml. + +ikiwiki escapes any html in svn commit logs to prevent other mischief. -- 2.26.2