git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jakub Narebski <jnareb@gmail.com>
To: git@vger.kernel.org
Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
Subject: [PATCH] gitweb: Sanitize title attribute in format_subject_html
Date: Fri, 22 May 2009 17:35:46 +0200	[thread overview]
Message-ID: <200905221735.48310.jnareb@gmail.com> (raw)

Replace control characters with question mark '?' (like in
chop_and_esc_str).


A little background: some web browsers turn on strict (and
unforgiving) XML validating mode for XHTML documents served using
application/xhtml+xml content type.  This means among others that
control characters are forbidden to appear in gitweb output.

CGI.pm does by default slight escaping (using simple_escape subroutine
from CGI::Util) of all _attribute_ values (depending on the value of
autoEscape, by default on).  This escaping, at least in CGI.pm version
3.10 (most current version at CPAN is 3.43), is minimal: only '"',
'&', '<' and '>' are escaped using named HTML entity references
(&quot;, &amp;, &lt; and &gt; respectively).  But simple_escape does
not do escaping of control characters such as ^X which are invalid in
XHTML (in strict mode).

If by some accident commit message do contain some control character
in first 50 characters (more or less) of first line of commit message,
and this line is longer than 50 characters (so gitweb shortens it for
display), then gitweb would put this control character in title
attribute (and CGI.pm would not remove them).  The tag _contents_ is
safe because it is escaped using esc_html() explicitly, and it
replaces control characters by their printable representation.


While at it: chop_and_escape_str doesn't need capturing group.

Noticed-by: Paul Gortmaker <paul.gortmaker@windriver.com>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This issue first appeared (with a wrong solution) month ago in
http://thread.gmane.org/gmane.comp.version-control.git/116755

I'm sorry Paul that it took so long to fix it.


This patch will be followed by 'lite' version, with minimal commit
message (this one is a bit long), and with failed attempt using
esc_attr.

 gitweb/gitweb.perl |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 05702e4..1e7e2d8 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1236,7 +1236,7 @@ sub chop_and_escape_str {
 	if ($chopped eq $str) {
 		return esc_html($chopped);
 	} else {
-		$str =~ s/([[:cntrl:]])/?/g;
+		$str =~ s/[[:cntrl:]]/?/g;
 		return $cgi->span({-title=>$str}, esc_html($chopped));
 	}
 }
@@ -1459,6 +1459,7 @@ sub format_subject_html {
 	$extra = '' unless defined($extra);
 
 	if (length($short) < length($long)) {
+		$long =~ s/[[:cntrl:]]/?/g;
 		return $cgi->a({-href => $href, -class => "list subject",
 		                -title => to_utf8($long)},
 		       esc_html($short) . $extra);
-- 
1.6.3.1

             reply	other threads:[~2009-05-22 15:36 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-05-22 15:35 Jakub Narebski [this message]
2009-05-22 15:39 ` [PATCH (lite)] gitweb: Sanitize title attribute in format_subject_html Jakub Narebski
2009-05-22 15:55 ` [RFC/PATCH (do not use)] gitweb: Introduce and use esc_attr to escape values of tag attributes Jakub Narebski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200905221735.48310.jnareb@gmail.com \
    --to=jnareb@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=paul.gortmaker@windriver.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).