+++ This bug was initially created as a clone of Bug #1305677 +++

Adam Willard reports the following XSS flaws in Satellite 5:

/rhn/admin/BunchDetail.do?label=cobbler-sync-bunch"<script>alert(1)</script>

/rhn/software/packages/NameOverview.do?package_name=sac">Test<script>alert(1)</script>&search_subscribed_channels=yes&channel_filter=539

/rhn/software/packages/NameOverview.do?package_name=sac">Test<script>alert(1)</script>&search_subscribed_channels=yes&channel_filter=539
/rhn/software/packages/NameOverview.do?package_name=sac">Test<script>alert(1)</script>&search_subscribed_channels=yes">test<script>alert(2)</script>&channel_filter=539"><script>alert(3)</script>

<input type="hidden" name="package_name" value="sac">Test<script>alert(1)</script>" />
<input type="hidden" name="search_subscribed_channels" value="yes">test<script>alert(2)</script>" />
<input type="hidden" name="channel_filter" value="539"><script>alert(3)</script>" />

--- Additional comment from Grant Gainey on 2016-02-15 16:08:43 EST ---

BunchDetail - rhn:toolbar and rl:listset tags need to stop trusting their input

Issues with "hidden":

396 total instances where we use either <html:hidden> or <input type="hidden>, and a value output with something like 'value="${param.ksid}"/> 

214 'unique':

(1305677_XSS) ~/git/satellite/java $ find . -name \*.jsp\* | xargs grep hidden | grep 'value=\"${' | grep -v 'c:out' | cut -f 2- -d: | sort -b -u | wc -l
214

105 <html:hidden>, 109 <input type="hidden">

If the html:hidden values are coming from form-vars, then they should be getting scrubbed already (need some more investigation to verify). That would leave us with 109 instances that we'd just have to track down and store 'correctly'. Perhaps build an RhnHiddenTag that encodes its value, always, and force its use everywhere we currently use <input type="hidden">?

--- Additional comment from Grant Gainey on 2016-02-16 16:45:45 EST ---

BunchDetail - #c2 is incorrect. The issue here is constructs like the following:

  <bean:message key="bunch.edit.jsp.toolbar" arg0="${label}"/>

where 'label' is set based on incoming parameter values:

  String bunchLabel = request.getParameter("label");
  request.setAttribute("label", bunchLabel);

To avoid the XSS attack, when/wherever we do BOTH of these things, setAttribute() must be called with a StringEscapeUtils.escapeHtml() around the value.

There are 196 instances in spacewalk we should check, to fix this kind-of problem.

RE the (over)use of <input type='hidden'> with unescaped values - I have a set of changes to HiddenInputTag.java, and a sed-script that converts all of our <input type="hidden"> to <rhn:hidden> instances, which enforce always using StringEscapeUtils.escapeHtml() on the value=. This needs more testing, to make sure it doesn't break anything.

--- Additional comment from Kurt Seifried on 2016-02-24 11:45:11 EST ---

Acknowledgments:

Name: Adam Willard of Raytheon Foreground Security