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 .= ' ' . &link("[$cvstree]", sprintf('%s/%s#dirlist', $scriptname, $query)); $retval .= $_; sub cvswebMarkup($$$) { print "Tag: <b>", $input{only_with_tag}, "</b><br>\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 '<a href="%s">%s</a>', hrefquote($url), $name; sub printLog($;$) { print "<a name=\"rev$_\"></a>"; print "<a name=\"$sym\"></a>"; print "<a name=\"$sym\"></a>"; sprintf( '%s?r1=%s%s', $scriptwhere, $_, htmlquote($barequery) ) print "Revision <b>$_</b>"; print "<i>", $author{$_}, "</i>\n"; print "<br>Branch: <b>", $link ? link_tags($revsym{$br}) : $revsym{$br}, print "<br>CVS Tags: <b>", $link ? link_tags($revsym{$_}) : $revsym{$_}, print "<br>Branch point for: <b>", $link ? link_tags($branchpoint{$_}) : $branchpoint{$_}, "</b>\n" print "<br>Changes since <b>$prev: $difflines{$_} lines</b sub doLog($) { print "Current tag: $input{only_with_tag}<br>\n"; print ">${_}</option>\n"; sub human_readable_diff($) { print "<h3 style=\"text-align: center\">Diff for /$where_nd between versio sub navigateHeader($$$$$) { <title>$path$filename - $title - $rev</title>$css print &link($backicon, "$swhere$query#rev$rev"); print "<b>Return to ", &link($filename, "$swhere$query#rev$rev"), sub chooseCVSRoot() { print "<input type=\"hidden\" name=\"$k\" value=\"$input {$k}\">\n" print "CVS Root: <b>[$cvstree]</b>"; sub download_link($$$;$) { print "><b>$textlink</b></a>"; sub html_header($) { <title>$title</title>
