CODEWITHSUNDEEP
c#exception
Eric
Eric

Reputation: 655

Is it necessary to wrap every exception at top level?

Today, someone told me that we should always wrap every exception at the top level of the framework. The reason is the original exception may contains sensitive information in the stacktrace or message, especially the stacktrace.

I don't know, is there any rules/principles/patterns?

Upvotes: 3

Views: 773

Answers (3)

Eric Lippert
Eric Lippert

Reputation: 660024

Today, someone told me that we should always wrap every exception at the top level of the framework.

"Always" seems a bit much.

Like any other design decision, you should consider the costs and the benefits.

Because the original exception may contains sensitive information in the stacktrace or message, especially the stacktrace.

Indeed; the exception could contain sensitive information, and the stack trace can be used by attackers.

are there any rules/principles/patterns?

Yes. Before you do anything else, particularly before you make a design or code change, make a threat model. You are asking a security question and therefore you absolutely positively have to understand the threats before you can devise a good strategy to mitigate the vulnerabilities.

The central questions answered by your threat model should be "what are the trust boundaries of my application? When and how does data cross a boundary? What vulnerabilities does this expose? What threats can an attacker make good on as a result?"

If you don't understand precisely what trust boundaries, vulnerabilities, threats and attackers are, then learn what those words mean before you try to design a security system to mitigate vulnerabilities to threats. "Writing Secure Code 2" is a good place to start. (Chapter 5 of my book on code security has some good advice on eliminating exception vulnerabilities, but it is long out of print. Maybe I'll put it up on the blog one of these days.)

Data can cross a trust boundary in either direction; an untrusted client might be sending malformed data to your server, and your server might be sending sensitive private data to the untrustworthy client.

The particular aspect of the threat model that your question specifically addresses is data in the form of exceptions. I kid you not, before we shipped .NET 1.0 you could actually get the framework to give you an exception whose text was something like "You do not have permission to determine the name of directory C:\foo". (Great. Thanks for letting me know. I'll be sure to not use that information to attack the user now.)

Obviously that one got fixed long before we shipped, but people do the moral equivalent of that every day. If you have data crossing a trust boundary, you should assume that the hostile user on the untrusted side is going to try to cause exceptions on the trusted side, and is going to try to learn as much about the system as possible from those exceptions. Do not make the attacker's job easier.

You asked whether all exceptions should be wrapped. Maybe. If in fact you have a problem -- if an exception containing sensitive data can cross a trust boundary, then maybe wrapping the exception is the right thing to do. But maybe it is not enough. Maybe you need to not throw an exception across the boundary at all, even if the exception can be santized. Maybe the right thing to do is to go on full red alert and say "hey, we got an unexpected exception caused by bad data from a potentially hostile third party, so let's (1) ban their IP, or (2) redirect them to the honeypot server, or (3) alert the security department, or (4) something else." What the right solution is depends on the threat, which you have not yet stated.

Like I said the first thing you need to do is model the threats. Don't go making security decisions without understanding the threats thoroughly.

Upvotes: 15

Kamil
Kamil

Reputation: 13931

Displaying stack trace for user in normal conditions is not best idea. Im using DEBUG constant set to "true" for debugging and developing, and set to "false" in normal conditions, look at that PHP example

DEFINE("DEBUG" true);

function soap_error($soapFault)
{
if (DEBUG)
    {
        // full message with stack including sensitive info
        echo '<p class="error">SOAP error:</p><br />';
        echo '<p>'.$soapFault.'</p><br />';
        echo '<hr />';
    }
    else
    {
        // only error message           
        echo '<p class="error">SOAP error:</p><br />';
        echo '<p>'.$soapFault->getMessage().'</p><br />';
        echo '<hr />';
    }
}

You can also use log files to save full error stacks.

In C#/.NET this is easier, because you have build configurations in Visual Studio and you can switch between them and use something like that:

public static void myerrorhandler(Exeption e) 
 {
 #if (DEBUG) // you have DEBUG and RELEASE configurations by default
 yourErrorMessageFunction("This is error message and stack " + e.toString()); 
 #else
 yourErrorMessageFunction("This is only message" + e.Message());
 #endif
 }

Visual Studio is adding and controlling DEBUG constant (its set in "Build configurations", you can customize it there).

Upvotes: -2

TomTom
TomTom

Reputation: 62093

Yes, there is a rule: it is stupid.

We need to know more details, but at the end:

  • You can and should have a top level handler for exceptions. This is not in your framework (i.e. wrapping top level) but attached to the appdomain (unhandled exception). This allows you to show an error message and write logs etc.

  • At the end you should propagate as much information as possible. The rule is "catch what you can handle, propagte waht yo ucan not".

  • Now it gets funny. "sensitive informatioN" is "information usefull and need3ed for debuggin". On top standards say "wrap an exception = the original goes into the inner exception property".

The framework should not expose sensitive infrmation to the user, but this is application specific (iis: custom error page, windowms etc.: last resort handler, not showing the user too many details because the user does not care anyway).

Upvotes: 0

Related Questions