From: joey <joey@0fa5a96a-9a0e-0410-b3b2-a0fd24251071>
Date: Sun, 2 Jul 2006 02:18:31 +0000 (+0000)
Subject: * Parse svn log as xml for improved utf8 and security. Note that this makes
X-Git-Url: https://sipb.mit.edu/gitweb.cgi/ikiwiki.git/commitdiff_plain/0bb605baf8702d1a5d1ee16f52b07840d88616e7?ds=sidebyside

* Parse svn log as xml for improved utf8 and security. Note that this makes
  ikiwiki depend on XML::Simple. Patch by Faidon Liambotis.
---

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 (<SVNLOG>) {
 		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 <joeyh@debian.org>  Sat,  1 Jul 2006 20:28:26 -0400
+ -- Joey Hess <joeyh@debian.org>  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.