OHDSI / WebAPI

OHDSI WebAPI contains all OHDSI services that can be called from OHDSI applications
Apache License 2.0
127 stars 166 forks source link

Unhandled Application Exceptions #1235

Closed konstjar closed 5 years ago

konstjar commented 5 years ago

Overview The Atlas application does not cleanly handle application exceptions, often presenting verbose errors to the end user. It was found that the Atlas application does not appear to handle unexpected application conditions. Some instances were constrained to simple error responses, as seen in the following:

POST /WebAPI/conceptset/ … … Cache-Control: no-transform ={"name":"Concept2","id":0} HTTP/1.1 400 Content-Length: 249 Unexpected character ('=' (code 61)): expected a valid value (number, String, array, object, 'true', 'false' or 'null') at [Source: org.glassfish.jersey.message.internal. ReaderInterceptorExecutor$UnCloseableInputStream@54b4db71; line: 1, column: 2]

Application Error Exposing Underlying Framework In other instances, the application response included a full application stack trace, including detailed internal information about the underlying application failure. These details could provide with insight into which parts of the application are in use when submitting malformed data and can often provide enough details. For instance, when discovering SQL injection within the application, these stack traces provided exact details regarding the structure of the malformed SQL statements:

HTTP/1.1 500 Connection: close Content-Length: 99033 … … }, "errorCode": 0, "sqlstate": "42601", "nextException": null, "message": "ERROR: syntax error at or near ";"\n Position: 761", "localizedMessage": "ERROR: syntax error at or near ";"\n Position: 761", "suppressed": [] }, "message": "StatementCallback; bad SQL grammar [select\n f.covariate_id,\n fr.covariate_name, \n ar.analysis_id,\n ar.analysis_name, \n ar.domain_id,\n ar.start_day,\n ar.end_day,\n fr.concept_id,\n\tc.concept_name,\n f.sum_value as count_value, \n f.average_value as stat_value\nfrom cdm_53_results.cohort_features f\njoin cdm_53_results.cohort_features_ref fr on fr.covariate_id = f.covariate_id and fr.cohort_definition_id = f.cohort_definition_id\nJOIN cdm_53_results.cohort_features_analysis_ref ar on ar.analysis_id = fr.analysis_id and ar.cohort_definition_id = fr.cohort_definition_id\nLEFT JOIN cdm_53.concept c on c.concept_id = fr.concept_id\nwhere f.cohort_definition_id = 10 AND f.average_value > .005 AND\n(lower(ar.domain_id) = lower('demographics';--'))\nORDER BY f.average_value DESC\n]; nested exception is org.postgresql.util.PSQLException: ERROR: syntax error at or near ";"\n Position: 761", "localizedMessage": "StatementCallback; bad SQL grammar [select\n f.covariate_id,\n fr.covariate_name, \n ar.analysis_id,\n ar.analysis_name, \n ar.domain_id,\n ar.start_day,\n ar.end_day,\n fr.concept_id,\n\tc.concept_name,\n f.sum_value as count_value, \n f.average_value as stat_value\nfrom cdm_53_results.cohort_features f\njoin cdm_53_results.cohort_features_ref fr on fr.covariate_id = f.covariate_id and fr.cohort_definition_id = f.cohort_definition_id\nJOIN cdm_53_results.cohort_features_analysis_ref ar on ar.analysis_id = fr.analysis_id and ar.cohort_definition_id = fr.cohort_definition_id\nLEFT JOIN cdm_53.concept c on c.concept_id = fr.concept_id\nwhere f.cohort_definition_id = 10 AND f.average_value > .005 AND\n(lower(ar.domain_id) = lower('demographics';--'))\nORDER BY f.average_value DESC\n]; nested exception is org.postgresql.util.PSQLException: ERROR: syntax error at or near ";"\n Position: 761", "suppressed": []

Recommendation(s) The Atlas application should ensure that all exceptions are handled cleanly, providing a method to capture error logs on the server, while presenting the user with a generic error.

ReferencesCWE-248: Uncaught Exception: https://cwe.mitre.org/data/definitions/248.html

chrisknoll commented 5 years ago

While I agree sending a raw error blob to the client is not friendly, shouldn't there we a way for the client to get to these details for error reporting purposes? The message provided in your example is helpful: ERROR: syntax error at or near ";"\n Position: 761, plus the query that caused the problem.

If the concern is revealing the database structure, security by obscurity isn't really security, and as an opensource project, people can just look up the table structures directly from the source.

anthonysena commented 5 years ago

This was closed with the 2.7.3 release. Comments in this thread were addressed in PR #1237.