Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add an option to display runtime errors #55

Closed
kirillg opened this issue Oct 3, 2011 · 7 comments
Closed

add an option to display runtime errors #55

kirillg opened this issue Oct 3, 2011 · 7 comments
Labels

Comments

@kirillg
Copy link

kirillg commented Oct 3, 2011

Would be great to have an equivalent of ASP.NET YSOD that displays two different versions by default (with runtime error info and without) and is configurable.

An equivalent of <customErrors mode=".."/>

Logs are great, but they serve different purpose and require extra key strokes.

@tjanczuk
Copy link
Owner

The idea is for iisnode to return the latest console output generated by the node.exe process plus any other useful diagnostic information in the body of the HTTP response whenever iisnode is forced to return an HTTP 500 today due to communication failure with node.exe. The idea behind this feature is to help with diagnosing issues during development. Imagine the node.js application being developed throwing an unhandled exception. The exception would be captured in the stderr which iisnode saves to a file on disk. Whenever iisnode fails to communicate with node.exe to process a new HTTP request, it would return an HTTP 500 (or convert it to 200) with the body containing the snapshot of the latest logs generated by the node.exe process.

@kirillg
Copy link
Author

kirillg commented Nov 18, 2011

Will this be controlled via @node_env / @loggingEnabled switches, or turned on always?

-----Original Message-----
From: Tomasz Janczuk [mailto:[email protected]]
Sent: Thursday, November 17, 2011 8:31 PM
To: Kirill Gavrylyuk
Subject: Re: [iisnode] add an option to display runtime errors (#55)

The idea is for iisnode to return the latest console output generated by the node.exe process plus any other useful diagnostic information in the body of the HTTP response whenever iisnode is forced to return an HTTP 500 today due to communication failure with node.exe. The idea behind this feature is to help with diagnosing issues during development. Imagine the node.js application being developed throwing an unhandled exception. The exception would be captured in the stderr which iisnode saves to a file on disk. Whenever iisnode fails to communicate with node.exe to process a new HTTP request, it would return an HTTP 500 (or convert it to 200) with the body containing the snapshot of the latest logs generated by the node.exe process.


Reply to this email directly or view it on GitHub:
#55 (comment)

@sidneylimafilho
Copy link

I agree with the main idea, but send the latest logs generated by node process as body in HTTP 500/200 would can be a security risk. (SQL Injection)

Another point is whether node process shutdown, on next request the iisnode start node again and again. In some moment the iisnode should stop to create node processes. (equals the apppool flag - rapidFailProtectionMaxCrashes)

@tjanczuk
Copy link
Owner

Can you explain the mechanics of the SQL injection attack in this context? The data flow here is from the server to the browser.

I agree we should consider adding iisnode feature similar to rapidFailProtectionMaxCrashes. Created #84 to track this.

@sidneylimafilho
Copy link

var db = require('buggyDriver'), http = require('http');
http.createServer(function(req, res){
    db.query("SELECT field1, field2 FROM Table1 WHERE ID = " + req.params.id, function(data){
        res.send('user', data);
    });
}).listen(process.env.PORT || 3000);
  • db.query fire a unhandled exception and node aborts, with the following message: "Query is invalid: 'SELECT field1, field2, field3, field4 FROM Table1 WHERE ID = 'foo' and ..."

If iisnode store this message in a file and after show on body of HTTP 500/200, then somebody can to explore it.

Can you understood what I thought?

@tjanczuk
Copy link
Owner

I understand. This isn't really SQL injection attack, it is information disclosure attack. There is always a trade off between usability in the development scenario (when everything runs on your own box and you want easy access to failure information), and security in production (where you don't want to leak any confidential data to the client).

In iisnode there are several web.config settings that control the level of information available to the client: iisnode/@loggingEnabled, iisnode/@debuggingEnabled, and iisnode/@devErrorsEnabled (the last one was just added when fixing #62). All are true by default (i.e. good for development scenario, bad for production). If you flip them all to false no information will leak to the client.

In the context of this issue (#55), we are discussing what information is sent back to the client when iisnode runs into an issue while @devErrorsEnabled==true and @loggingEnabled==true. So we should be liberal in disclosing server side details in the messages back to the client.

@tjanczuk
Copy link
Owner

The fix is to generate friendly HTTP 200 responses with HTML entity body for error conditions that would otherwise result in iisnode returning an HTTP 5xx reponse when iisnode/@devErrorsEnabled setting in web.config is 'true' (which is the default). In production this setting should be set to 'false'.

The HTML response contains the HTTP status code and reason that would be sent otherwise, the HRESULT that captures the error condition, and the last 64k of the output generated by the node.exe process to stdout or stderr, if anything was generated at all. Logging behavior of node.exe depends on a separate iisnode/@loggingEnabled property (by default 'true').

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants