How often do you just use an HTML comment to remove old code, or new functionality that isn’t ready yet? Are HTML comments effective for ASP.Net server controls? From a pure development context, they probably are. When we factor in security, they no longer provide the functionality that was intended. This post will explain an issue with how ASP.Net handles this situation and why it is not sufficient from a security perspective.

I am going to use a very simplistic example to make it easier to understand and to save space. Please do not let this simple example downplay the significance of this issue. In this example, I have added two label controls and a button control. I will walk through a few different scenarios to explain what is happening. Here is what the relevant part of the html page looks like:

  1: <asp:Content ID="BodyContent" runat="server" ContentPlaceHolderID="MainContent">
  2:     <h2>
  3:         HTML Comment Test Page
  4:     </h2>
  5:     <p>
  6:         <asp:Label ID="lblUserName" runat="server" />
  7:         <asp:Label ID="lblAcctNum" runat="server" />
  8:         <asp:Button ID="cmdSubmit" runat="server" Text="Submit"
  9:             onclick="cmdSubmit_Click" />
 10:     </p>
 11: </asp:Content>

Below is the relevant code from the default.aspx.cs code behind file.  This is the code that contains the button click event that will be important throughout this post.

  1: protected void Page_Load(object sender, EventArgs e)
  2: {
  3:     lblUserName.Text = "joeUser";
  4:     lblAcctNum.Text = "933-3222222-939239";
  5: }
  6: protected void cmdSubmit_Click(object sender, EventArgs e)
  7: {
  8:     lblUserName.Text = "This shouldn't have happened.";
  9: }

When we run the page, the following output is exactly what we expect.  The labels are populated with their respected values and the button is there. 

  1: <h2>
  2:     HTML Comment Test Page
  3: </h2>
  4: <p>
  5:     <span id="MainContent_lblUserName">joeUser</span>
  6:     <span id="MainContent_lblAcctNum">933-3222222-939239</span>
  7:     <input type="submit" name="ctl00$MainContent$cmdSubmit" value="Submit" id="MainContent_cmdSubmit" />
  8: </p>

I would like to call out separately the EventValidation value.  It is beyond the scope of this post to fully explain EventValidation within .Net, but the short explanation is that it is used to only allow expected events to be triggered on the server.  The value is a hash of the allowed values and events.  Here is the value for this page:

  1: /wEWAgLFs4viAgKP5u6DCOTYhw/mo3AHFQHnB8XMEMHlrATJQZUNTjLevOMdQoay

Now lets comment out the button control and a label control and see what happens.  I will comment out the account number because I realized there are some problems with it.  It is displaying sensitive information and it is vulnerable to cross-site scripting.  I want to remove the button, because it is old functionality and I don’t want anyone to be able to perform that action going forward.  Here is what the new HTML data looks like:

  1: <h2>
  2:     HTML Comment Test Page
  3: </h2>
  4: <p>
  5:     <asp:Label ID="lblUserName" runat="server" />
  6:
  7:     <!--<asp:Label ID="lblAcctNum" runat="server" />-->
  8:     <!--<asp:Button ID="cmdSubmit" runat="server" Text="Submit"
  9:         onclick="cmdSubmit_Click" />-->
 10: </p>

It is important to note that I did not make any changes to the code behind file, because it was a) easier to update the html document and b) it doesn’t require me to change compiled code.  Although neither of these are good reasons, they are just used for the example.

Here is what the new HTML output looks like:

  1: <h2>
  2:      HTML Comment Test Page
  3: </h2>
  4: <p>
  5:      <span id="MainContent_lblUserName">joeUser</span>
  6:
  7:      <!--<span id="MainContent_lblAcctNum">933-3222222-939239</span>-->
  8:      <!--<input type="submit" name="ctl00$MainContent$cmdSubmit" value="Submit" id="MainContent_cmdSubmit" />-->
  9: </p>

The good news is that the elements are commented out, as we expected, but do you see a problem?  On line 7, the account number is still being populated and on line 8 the button was processed as well to a proper HTML button, rather than the asp:control that we commented out.  I would have expected that the controls would not have been processed because they were commented out. 

The .Net framework does not detect that the ASP.Net server controls are within HTML comments and still processes them as if they were not commented out.  Now, that sensitive data I was trying to remove, is just not displayed on the page, but still available in the source of the page.  All a user would have to do is View Source to see it. 

Remember I also mentioned I wanted to remove this label because it was vulnerable to cross-site scripting.  Well, unfortunately, it still is.  Just because it lives inside of HTML comments doesn’t mean if the proper values are sent down it is still protected.  It is possible for an attacker to submit data that will break out of the comment and then add their own execution steps.

Also, notice that the button looks valid.  What would happen if the end user decided to remove the comments around the button?  In this case, the button would still fire as it did in the first example.  There are other ways to trigger this button, but re-enabling it with a proxy or browser add-in is the easiest.  Lets take another look at our EventValidation value:

  1: /wEWAgLFs4viAgKP5u6DCOTYhw/mo3AHFQHnB8XMEMHlrATJQZUNTjLevOMdQoay

This value is the same as the last time we checked it before commenting out the button control.   Since it is the same, we know that the same events are available to us and the button will be accepted.  This could cause a problem if the code is not ready for prime time, or has been removed due to issues of some sort.

A better way to resolve this issue is to set the Visible property for each of the server controls you no longer want to exist on the page, or just remove them if you are using source control.   By setting the visible property to “False”, the .Net framework will not process that control and send it to the browser.  This also effects the EventValidation.  Lets look and see what happens when we set the Visible property in the HTML.

  1: <h2>
  2:     HTML Comment Test Page
  3: </h2>
  4: <p>
  5:     <asp:Label ID="lblUserName" runat="server" />
  6:
  7:     <asp:Label ID="lblAcctNum" Visible="false" runat="server" />
  8:     <asp:Button ID="cmdSubmit" runat="server" Visible="false" Text="Submit"
  9:         onclick="cmdSubmit_Click" />
 10: </p>

Here is the output:

  1: <p>
  2:     <span id="MainContent_lblUserName">joeUser</span>
  3:
  4:    
  5:    
  6: </p>

Notice how the button and label are no longer being output to the browser at all.   This is much better and more secure.  Lets check the EventValidation value just to make sure it has changed. Oh, wait, because there are no other events on this page, EventValidation not longer exists.  That means that no events would be accepted by our page.  If other events existed, or if a listbox existed, then our value would have been different.  If we tried to then submit the button click event, an ASP.Net error would have been raised. 

As you can see, there are some issues when just using HTML comments for your server controls.   ASP.Net has an issue, maybe a bug, that doesn’t detect this behavior and processes the controls as if they were allowed.  Remember, it is easy to re-enable a commented out control, so make sure you avoid that situation.  Just because a controls is commented, doesn’t mean it will represent a security risk, but any instance of this should be examined for potential security problems.