Potential Cross-site Scripting Attack when entering in a Color

The developer division is currently in the middle of a security push. Part of this is to review all code with other developers, and associated QA person and program manager. Yesterday, we were reviewing WebPartManager. We pay extra focus on the rendering logic, to make sure we are not introducing any cross-site scripting attacks. This is especially applicable to WebParts, given we have Web UI to allow the page personalizer and customizer to change the content of the page.

One of the things we discussed was rendering of colors. Our HtmlTextWriter encodes some attributes, and does not encode some other attributes (eg. integers), because they can't possibly contain text or script to induce an attack. We lump colors into the same bucket. Shouldn't an RGB triplet be harmless?

The answer is that it depends... specifically on how the color object was intiialized. If one uses Color.FromName(name), the name is preserved as-is if it does not match a known color, or can't be parsed as an RGB triplet. This is a problem if the name is coming in as part of the request. Consider this code:

  <asp:TextBox runat="server" id="colorTextBox" />
  <asp:Panel runat="server" id="colorPanel" />
  <asp:Button runat="server" id="okButton" Text="OK" onclick="okButton_Click" />
  <script runat="server">
  void okButton_Click(object sender, EventArgs e) {
      colorPanel.BackColor = Color.FromName(colorTextBox.Text);
  }
  </script>

So if the page user enters in "expression(alert('hi'))" for the color, this will essentially execute the script! This is basically equivalent to the classic and most basic form of cross-site scripting attacks. The rendered HTML looks like:

  <div style="background-color: expression(alert('hi'))" />

So the first question was whether our built-in PropertyGridEditorPart allowed one to enter such colors (if a particular WebPart had personalizable Color property). The answer thankfully is no. The reason?

We happen to use TypeConverters to convert from string to property type. Turns out the ColorConverter does validate that the color is a valid color. So if I replace the Color.FromName() call to use a TypeConverter instead, I am safe. For example:

  string colorText = colorTextBox.Text;
  Color color = TypeDescriptor.GetConverter(typeof(Color)).ConvertFromString(colorText);
  colorPanel.BackColor = color;

So the first recommendation is to use the TypeConverter, and not Color.FromName should you accept color values from the user of your Web application. Alternatively check to make sure the color is a valid color. This is all in vein with always validating user input first.

The next obvious question is could we in ASP.NET do something about it? This is still an open question, but it is unclear what specifically to do. Even if we switched the HtmlTextWriter to automatically encode color attributes, it only covers values that contain things like quotes, '&' etc. and you can still enter in expressions and a whole range of script that might not get encoded. Perhaps, we could also encode '(' and ')' and that would cover many more script scenarios, but then they would be encoded in any attribute, and not just colors. I'd love to hear any thoughts that people might have.

Side note: WebPartManager is a pretty huge beast J... second only to Page within System.Web.UI.* in terms of code size!