Re: [Re: XSS vuln in cvsweb]
On Fri, Mar 22, 2019 at 01:41:27AM +0100, Ingo Schwarze wrote:
> I committed the patches from both of you to
>
> http://mandoc.bsd.lv/cgi-bin/cvsweb/?cvsroot=cvsweb
> http://mandoc.bsd.lv/cvsweb/
Thanks! I hadn't actually realized there was an upstream past the
OpenBSD ports tree. Good to know.
> in preparation for rolling a new release, then briefly looked at
> the script myself. My impression is there are huge numbers of
> additional candidates for missing escaping, see the incomplete list
> below (which may of course also contain some false positives). I'm
> not yet sure what to do about it (don't waste your time right now
> trying to fix all the potential issues below).
Yes, I believe the rest are all either actually safe due to some sort of
additional validation, or come from the cvs repository and so are less
likely to be malicious (or at least not significantly more malicious
than anything else being served).
> Maybe just fix the most important ones, check and apply the additional
> idea (adding some HTTP header) from sthen@, release 2.1.1 even if
> it is not perfect, update the port, then move on to the 3.0 branch,
> cleaning up things for real over there, aiming for a later 3.1.1
> release?
This is definitely an old codebase with lots of room for improvements.
If there is already work underway to improve that I certainly wouldn't
want to put too much work into patching up an old one only to have to
redo that work. I think patching up the important things and getting a
bugfix out is great with more improvements on a single branch.
I think I only see two spots below where the input could come from
someplace and may not be completely validated, the one in cvswebMarkup
and in chooseCVSRoot.
> That way, we would quickly get a 2.1.1 release that everybody could
> update to without too much hassle, whereas updating to 3.1.1 might
> not be easy for some, and besides releasing 3.1.1 will certainly
> not be quick. At the same time, that plan would avoid wasting too
> much time on 2.1.1. Not sure yet...
>
> Yours,
> Ingo
>
>
>
> Additional candidates where escaping is likely missing:
> ---
> sub clickablePath($$) {
> $retval = "[$cvstree]";
> $retval .= ' ' . ("[$cvstree]",
> sprintf('%s/%s#dirlist', $scriptname, $query));
$cvstree should be safe since it has to be something set in %CVSROOT via
@CVSrepositories in the config file. While I suppose it's possible for
that to contain stuff that needs escaping, it's unlikely to be
malicious.
if ($input{cvsroot} && $CVSROOT{$input{cvsroot}}) {
$cvstree = $input{cvsroot};
> $retval .= $_;
The current dir, again controlled by the content of the cvs repository
is probably fine.
> sub cvswebMarkup($$$) {
> print "Tag: ", $input{only_with_tag}, "\n"
This one should be fixed, and I see it is. Sorry about missing that one.
> sub doDiff($$) {
> print $_;
I believe this can only be reached after a text/plain header, not sure
we need to escape those.
> sub getDirLogs($$@) {
> print "$state:$_" if ($verbose);
I believe that $verbose causes things to print all sorts of stuff useful
only when debugging, if we need to sanitize that output it seemed like a
lot of work for something that was only useful when actually coding on
cvsweb. If I misunderstood $verbose, which is totally possible, but I
don't think it's something that works in "production".
> print "$filename $rev Wanted: $revwanted ",
> "Revbranch: $revbranch Branch: $branch ",
> "Branchpoint: $branchpoint\n"
> "File revision $rev found for
> branch
> $branch\n"
> print
> "File info $rev found for
> $filename
>
> sub readLog($;$) {
> print if ($verbose);
> print "R:", $_ if ($verbose);
> print "D:", $_ if ($verbose);
> print "L:", $_ if ($verbose);
> print "E:", $_ if ($verbose);
>
> sub link($$) {
> sprintf '%s', hrefquote($url), $name;
I think this is as designed and we'd have to make sure $name is escaped
as necessary before calling `link`.
# Note that this doesn't htmlquote the first argument...
> sub printLog($;$) {
> print "";
> print "";
> print "";
> sprintf(
> '%s?r1=%s%s', $scriptwhere,
> $_, htmlquote($barequery)
> )
> print "Revision $_";
> print "", $author{$_},
Re: [Re: XSS vuln in cvsweb]
Hi,
Peter J. Philipp wrote on Sat, Mar 16, 2019 at 07:52:23AM +0100:
> On Fri, Mar 15, 2019 at 05:22:47PM -0700, Andrew Hewus Fresh wrote:
>> I looked this over and updated the patch to be against the port. It
>> seems to be good and I only found a couple other places that needed to
>> be escaped, the "stickyvars" section and the tr1/tr2 inputs in doLog,
>> although r1, r2, tr1 and tr2 are part of "unsafevars" so their content
>> is pretty limited already.
>>
>> It was a pretty quick look so I don't doubt that there are more, and I
>> didn't actually get a chance to test it out, so hopefully someone else
>> can.
> Thanks for the help! I have applied your patch and it went cleanly (commands
> were cd /usr/ports/devel/cvsweb; patch -p0 < cvsweb.patch), I then rebuilt
> the port with "make reinstall" no problem there either. The CGI is the newly
> built CGI as discovered with ls. A quick test shows that it applied the
> XSS escapes.
I committed the patches from both of you to
http://mandoc.bsd.lv/cgi-bin/cvsweb/?cvsroot=cvsweb
http://mandoc.bsd.lv/cvsweb/
in preparation for rolling a new release, then briefly looked at
the script myself. My impression is there are huge numbers of
additional candidates for missing escaping, see the incomplete list
below (which may of course also contain some false positives). I'm
not yet sure what to do about it (don't waste your time right now
trying to fix all the potential issues below).
Maybe just fix the most important ones, check and apply the additional
idea (adding some HTTP header) from sthen@, release 2.1.1 even if
it is not perfect, update the port, then move on to the 3.0 branch,
cleaning up things for real over there, aiming for a later 3.1.1
release?
That way, we would quickly get a 2.1.1 release that everybody could
update to without too much hassle, whereas updating to 3.1.1 might
not be easy for some, and besides releasing 3.1.1 will certainly
not be quick. At the same time, that plan would avoid wasting too
much time on 2.1.1. Not sure yet...
Yours,
Ingo
Additional candidates where escaping is likely missing:
---
sub clickablePath($$) {
$retval = "[$cvstree]";
$retval .= ' ' . ("[$cvstree]",
sprintf('%s/%s#dirlist', $scriptname, $query));
$retval .= $_;
sub cvswebMarkup($$$) {
print "Tag: ", $input{only_with_tag}, "\n"
sub doDiff($$) {
print $_;
sub getDirLogs($$@) {
print "$state:$_" if ($verbose);
print "$filename $rev Wanted: $revwanted ",
"Revbranch: $revbranch Branch: $branch ",
"Branchpoint: $branchpoint\n"
"File revision $rev found for branch
$branch\n"
print
"File info $rev found for $filename
sub readLog($;$) {
print if ($verbose);
print "R:", $_ if ($verbose);
print "D:", $_ if ($verbose);
print "L:", $_ if ($verbose);
print "E:", $_ if ($verbose);
sub link($$) {
sprintf '%s', hrefquote($url), $name;
sub printLog($;$) {
print "";
print "";
print "";
sprintf(
'%s?r1=%s%s', $scriptwhere,
$_, htmlquote($barequery)
)
print "Revision $_";
print "", $author{$_}, "\n";
print "Branch: ", $link ? link_tags($revsym{$br}) : $revsym{$br},
print "CVS Tags: ", $link ? link_tags($revsym{$_}) : $revsym{$_},
print "Branch point for: ",
$link ? link_tags($branchpoint{$_}) : $branchpoint{$_}, "\n"
print
"Changes since $prev: $difflines{$_} lines\n";
print ">${_}\n";
sub human_readable_diff($) {
print
"Diff for /$where_nd between versio
sub navigateHeader($) {
$path$filename - $title - $rev$css
print ($backicon, "$swhere$query#rev$rev");
print "Return to ", ($filename, "$swhere$query#rev$rev"),
sub chooseCVSRoot() {
print "\n"
print "CVS Root: [$cvstree]";
sub download_link($$$;$) {
print ">$textlink";
sub html_header($) {
$title
Re: [Re: XSS vuln in cvsweb]
On Fri, Mar 15, 2019 at 05:22:47PM -0700, Andrew Hewus Fresh wrote: > > I have produced the patch with 'diff -u cvsweb.orig cvsweb' directly in the > > /var/www/cgi-bin directory. Credit goes to Ezio Paglia for finding this XSS > > vuln. Also the cvsweb at openbsd.org is affected and can be checked with: > > I looked this over and updated the patch to be against the port. It > seems to be good and I only found a couple other places that needed to > be escaped, the "stickyvars" section and the tr1/tr2 inputs in doLog, > although r1, r2, tr1 and tr2 are part of "unsafevars" so their content > is pretty limited already. > > It was a pretty quick look so I don't doubt that there are more, and I > didn't actually get a chance to test it out, so hopefully someone else > can. > > > https://cvsweb.openbsd.org/src/sbin/clri/clri.c?f=%22%3E%3Cscript%3Ealert(%27XSS%27)%3C/script%3E > > > > in chrome the XSS check activates immediately, I don't know what firefox > > does. > > With NoScript it throws up a big error saying there was an attempted > XSS, without NoScript it throws up an alert box saying "XSS". > Index: Makefile ... Hi, Thanks for the help! I have applied your patch and it went cleanly (commands were cd /usr/ports/devel/cvsweb; patch -p0 < cvsweb.patch), I then rebuilt the port with "make reinstall" no problem there either. The CGI is the newly built CGI as discovered with ls. A quick test shows that it applied the XSS escapes. Best Regards, -peter
Re: [Re: XSS vuln in cvsweb]
On Fri, Mar 15, 2019 at 02:25:35PM +0100, Peter J. Philipp wrote:
> I have have created a patch for cvsweb port that needs review and help in
> getting it into the port itself. I'd like to apologize to Marc Espie for
> contacting him regarding this port based on his last check-in on this port,
> and thanks to Stuart Henderson for directing me here.
>
> The patch changed since my last submission to misc@ and since I am a complete
> newbie in perl this would need a pro to look at it whether it's correct.
>
> I have produced the patch with 'diff -u cvsweb.orig cvsweb' directly in the
> /var/www/cgi-bin directory. Credit goes to Ezio Paglia for finding this XSS
> vuln. Also the cvsweb at openbsd.org is affected and can be checked with:
I looked this over and updated the patch to be against the port. It
seems to be good and I only found a couple other places that needed to
be escaped, the "stickyvars" section and the tr1/tr2 inputs in doLog,
although r1, r2, tr1 and tr2 are part of "unsafevars" so their content
is pretty limited already.
It was a pretty quick look so I don't doubt that there are more, and I
didn't actually get a chance to test it out, so hopefully someone else
can.
> https://cvsweb.openbsd.org/src/sbin/clri/clri.c?f=%22%3E%3Cscript%3Ealert(%27XSS%27)%3C/script%3E
>
> in chrome the XSS check activates immediately, I don't know what firefox does.
With NoScript it throws up a big error saying there was an attempted
XSS, without NoScript it throws up an alert box saying "XSS".
Index: Makefile
===
RCS file: /cvs/ports/devel/cvsweb/Makefile,v
retrieving revision 1.60
diff -u -p -r1.60 Makefile
--- Makefile4 Sep 2018 12:46:10 - 1.60
+++ Makefile16 Mar 2019 00:17:58 -
@@ -3,7 +3,7 @@
COMMENT= CGI script to browse CVS repository trees
DISTNAME= cvsweb-2.0.6
-REVISION= 26
+REVISION= 27
CATEGORIES=devel www
HOMEPAGE= http://www.freebsd.org/projects/cvsweb.html
Index: patches/patch-cvsweb_cgi
===
RCS file: /cvs/ports/devel/cvsweb/patches/patch-cvsweb_cgi,v
retrieving revision 1.13
diff -u -p -r1.13 patch-cvsweb_cgi
--- patches/patch-cvsweb_cgi7 Apr 2013 20:07:24 - 1.13
+++ patches/patch-cvsweb_cgi16 Mar 2019 00:17:58 -
@@ -1,6 +1,7 @@
$OpenBSD: patch-cvsweb_cgi,v 1.13 2013/04/07 20:07:24 naddy Exp $
cvsweb.cgi.origThu Sep 26 22:56:05 2002
-+++ cvsweb.cgi Sun Apr 7 14:15:55 2013
+Index: cvsweb.cgi
+--- cvsweb.cgi.orig
cvsweb.cgi
@@ -1,4 +1,4 @@
-#!/usr/bin/perl -wT
+#!/usr/bin/perl -w
@@ -37,7 +38,18 @@ $OpenBSD: patch-cvsweb_cgi,v 1.13 2013/0
);
@LOGSORTKEYS = qw(cvs date rev);
-@@ -2014,20 +2009,6 @@ sub doDiff($$) {
+@@ -1003,8 +998,9 @@ if (-d $fullname) {
+ if (scalar %tags || $input{only_with_tag}) {
+ print "\n";
+ foreach my $var (@stickyvars) {
++ my $tmpvar = htmlquote($input{$var});
+ print
+- "\n"
++ "\n"
+ if (defined($input{$var})
+ && (!defined($DEFAULTVALUE{$var})
+ || $input{$var} ne $DEFAULTVALUE{$var})
+@@ -2014,20 +2010,6 @@ sub doDiff($$) {
my @difftype = @{$difftype->{'opts'}};
my $human_readable = $difftype->{'colored'};
@@ -58,7 +70,25 @@ $OpenBSD: patch-cvsweb_cgi,v 1.13 2013/0
if ($human_readable) {
if ($hr_ignwhite) {
push @difftype, '-w';
-@@ -2658,7 +2639,7 @@ sub printLog($;$) {
+@@ -2631,7 +2613,7 @@ sub printLog($;$) {
+ sprintf(
+ '%s/%s?annotate=%s%s', $scriptname,
+ urlencode($where), $_,
+- $barequery
++ htmlquote($barequery)
+ )
+ );
+ }
+@@ -2644,7 +2626,7 @@ sub printLog($;$) {
+ '[select for diffs]',
+ sprintf(
+ '%s?r1=%s%s', $scriptwhere,
+- $_, $barequery
++ $_,
htmlquote($barequery)
+ )
+ );
+ } else {
+@@ -2658,7 +2640,7 @@ sub printLog($;$) {
if (/^1\.1\.1\.\d+$/) {
print " (vendor branch)";
}
@@ -67,3 +97,57 @@ $OpenBSD: patch-cvsweb_cgi,v 1.13 2013/0
my ($est) = $mytz[(localtime($date{$_}))[8]];
print ", ", scalar localtime($date{$_}), " $est (";
} else {
+@@ -2847,7 +2829,7 @@ sub doLog($) {
+
+
Re: [Re: XSS vuln in cvsweb]
On 2019/03/15 16:28, Peter J. Philipp wrote: > would this help any? > > https://people.freebsd.org/~scop/cvsweb/ > > There is subsequent versions. Those are the 13 year old ones that Ingo mentioned. > Regards, > > -peter > > On 3/15/19 4:05 PM, Ingo Schwarze wrote: > > Hi, > > > > the trouble with cvsweb is that it is important OpenBSD project > > infrastructure (consider cvsweb.openbsd.org) that has been abandoned > > upstream 13 years ago, our version is 16 years old, and the port > > has no maintainer. Does anybody consider it funny to run a software > > in production that is closely related to version control, but > > (according to my knowledge) is not currently under version control > > itself? > > > > Given that i'm still using it on bsd.lv, too, i'm willing to host > > a CVS repo for it and the release tarballs (historic and future) > > on bsd.lv, pick up upstream maintenance, and also maintain the port. > > > > Does that sound reasonable to people round here? > > If so, does anyone know whether a copy of the original CVS repository > > that it resided in still exists somewhere? It seems to have vanished > > from https://svnweb.freebsd.org/base/projects/ ... > > > > I don't consider the XSS urgent, but it would of course get fixed > > in the process. If people wanted, they could test and commit patches > > to the port beforehand, but i'm not sure it's needed. > > > > Yours, > >Ingo
Re: [Re: XSS vuln in cvsweb]
would this help any? https://people.freebsd.org/~scop/cvsweb/ There is subsequent versions. Regards, -peter On 3/15/19 4:05 PM, Ingo Schwarze wrote: Hi, the trouble with cvsweb is that it is important OpenBSD project infrastructure (consider cvsweb.openbsd.org) that has been abandoned upstream 13 years ago, our version is 16 years old, and the port has no maintainer. Does anybody consider it funny to run a software in production that is closely related to version control, but (according to my knowledge) is not currently under version control itself? Given that i'm still using it on bsd.lv, too, i'm willing to host a CVS repo for it and the release tarballs (historic and future) on bsd.lv, pick up upstream maintenance, and also maintain the port. Does that sound reasonable to people round here? If so, does anyone know whether a copy of the original CVS repository that it resided in still exists somewhere? It seems to have vanished from https://svnweb.freebsd.org/base/projects/ ... I don't consider the XSS urgent, but it would of course get fixed in the process. If people wanted, they could test and commit patches to the port beforehand, but i'm not sure it's needed. Yours, Ingo
Re: [Re: XSS vuln in cvsweb]
On 2019/03/15 16:05, Ingo Schwarze wrote:
> Hi,
>
> the trouble with cvsweb is that it is important OpenBSD project
> infrastructure (consider cvsweb.openbsd.org) that has been abandoned
> upstream 13 years ago, our version is 16 years old, and the port
> has no maintainer. Does anybody consider it funny to run a software
> in production that is closely related to version control, but
> (according to my knowledge) is not currently under version control
> itself?
>
> Given that i'm still using it on bsd.lv, too, i'm willing to host
> a CVS repo for it and the release tarballs (historic and future)
> on bsd.lv, pick up upstream maintenance, and also maintain the port.
>
> Does that sound reasonable to people round here?
> If so, does anyone know whether a copy of the original CVS repository
> that it resided in still exists somewhere? It seems to have vanished
> from https://svnweb.freebsd.org/base/projects/ ...
Definitely.
> I don't consider the XSS urgent, but it would of course get fixed
> in the process. If people wanted, they could test and commit patches
> to the port beforehand, but i'm not sure it's needed.
I wondered about doing something like this. Untested (I don't have an
installation handy) but it should disable inline scripts and some other
types of content, while still permitting inline css that cvsweb needs.
Index: cvsweb.cgi
--- cvsweb.cgi.orig
+++ cvsweb.cgi
@@ -3572,8 +3572,11 @@ sub http_header(;$) {
if ($is_mod_perl) {
Apache->request->content_type($content_type);
+ Apache->request->header_out(
+ "Content-Security-Policy" => "default-src 'self';
style-src 'self' 'unsafe-inline';");
} else {
print "Content-Type: $content_type\r\n";
+ print "Content-Security-Policy: default-src 'self'; style-src
'self' 'unsafe-inline';\r\n";
}
if ($allow_compress && $maycompress) {
Re: [Re: XSS vuln in cvsweb]
Hi, the trouble with cvsweb is that it is important OpenBSD project infrastructure (consider cvsweb.openbsd.org) that has been abandoned upstream 13 years ago, our version is 16 years old, and the port has no maintainer. Does anybody consider it funny to run a software in production that is closely related to version control, but (according to my knowledge) is not currently under version control itself? Given that i'm still using it on bsd.lv, too, i'm willing to host a CVS repo for it and the release tarballs (historic and future) on bsd.lv, pick up upstream maintenance, and also maintain the port. Does that sound reasonable to people round here? If so, does anyone know whether a copy of the original CVS repository that it resided in still exists somewhere? It seems to have vanished from https://svnweb.freebsd.org/base/projects/ ... I don't consider the XSS urgent, but it would of course get fixed in the process. If people wanted, they could test and commit patches to the port beforehand, but i'm not sure it's needed. Yours, Ingo
[Re: XSS vuln in cvsweb]
Hi,
I have have created a patch for cvsweb port that needs review and help in
getting it into the port itself. I'd like to apologize to Marc Espie for
contacting him regarding this port based on his last check-in on this port,
and thanks to Stuart Henderson for directing me here.
The patch changed since my last submission to misc@ and since I am a complete
newbie in perl this would need a pro to look at it whether it's correct.
I have produced the patch with 'diff -u cvsweb.orig cvsweb' directly in the
/var/www/cgi-bin directory. Credit goes to Ezio Paglia for finding this XSS
vuln. Also the cvsweb at openbsd.org is affected and can be checked with:
https://cvsweb.openbsd.org/src/sbin/clri/clri.c?f=%22%3E%3Cscript%3Ealert(%27XSS%27)%3C/script%3E
in chrome the XSS check activates immediately, I don't know what firefox does.
Patch after end of signature and forwarded message:
-peter
- Forwarded message from Stuart Henderson -
Date: Fri, 15 Mar 2019 12:16:06 - (UTC)
From: Stuart Henderson
To: m...@openbsd.org
Subject: Re: XSS vuln in cvsweb
User-Agent: slrn/1.0.2 (OpenBSD)
On 2019-03-15, Peter J. Philipp wrote:
> Hi all,
>
> I have been notified by a wonderful security researcher that my site was
> vulnerable to XSS attacks. The first one was on software I wrote, and the
> second one was on software I got from OpenBSD ports. Not sure if I should
> be writing this to the ports mailing list though.
>
> I have written Marc Espie with a patch that I produced for cvsweb, but
> haven't heard from him in 11 hours so I want to get this out to everyone.
Yes, it should go to the ports mailing list. Check the "maintainer" line
in "pkg_info cvsweb". I don't know why you would send it to espie@.
- End forwarded message -
--- cvsweb.orig Thu Mar 14 18:30:06 2019
+++ cvsweb Fri Mar 15 10:23:05 2019
@@ -998,8 +998,9 @@
if (scalar %tags || $input{only_with_tag}) {
print "\n";
foreach my $var (@stickyvars) {
+ my $tmpvar = htmlquote($input{$var});
print
- "\n"
+ "\n"
if (defined($input{$var})
&& (!defined($DEFAULTVALUE{$var})
|| $input{$var} ne $DEFAULTVALUE{$var})
@@ -2612,7 +2613,7 @@
sprintf(
'%s/%s?annotate=%s%s', $scriptname,
urlencode($where), $_,
- $barequery
+ htmlquote($barequery)
)
);
}
@@ -2625,7 +2626,7 @@
'[select for diffs]',
sprintf(
'%s?r1=%s%s', $scriptwhere,
- $_, $barequery
+ $_,
htmlquote($barequery)
)
);
} else {
@@ -2828,7 +2829,7 @@
foreach (@stickyvars) {
printf('', $_,
- $input{$_})
+ htmlquote($input{$_}))
if (defined($input{$_})
&& ((!defined($DEFAULTVALUE{$_})
|| $input{$_} ne $DEFAULTVALUE{$_}) && $input{$_} ne ""));
@@ -3267,7 +3268,7 @@
join ('', $scriptname,
urlencode($wherepath),
(!$last || $lastslash ? '/' : ''),
- $query,
+ htmlquote($query),
(!$last || $lastslash ? "#dirlist" : "")
));
} else {# do not make a link to the current dir
@@ -3508,6 +3509,7 @@
# Special Characters; RFC 1866
s/&//g;
s/\"//g;
+ s/%22//g;
s///g;
