As a security consultant, I perform assessments across a wide variety of applications.
Throughout the applications I've tested, I've found it's common for them to suffer from some form of inadequate exception handling and logging.
Logging and monitoring are often-overlooked areas, and due to increased threats against web applications, they've been added to the OWASP Top 10 as the new number ten issue, under the name "Insufficient Logging and Monitoring."
So what’s the problem here? Well, let’s take a look.
Logs? Who Needs Logs?
To start off, why do we even use logging? What’s the point?
Not only is proper logging useful for debugging applications, but it also has serious implications for compliance and many benefits for forensics and incident response.
How do you know if someone is running a vulnerability scanner against your application?
Or is attempting a brute force authentication attack to try and access user accounts? All of this is good to know, but there are other subtle things as well.
The majority of successful attacks start with an attacker who probes the application and looks for weak points.
The more an attacker can probe the application, the higher the chance that the attacker will find and successfully exploit the application.
Attackers rely on being able to go unnoticed, and since the breach detection rate is an average of 191 days, the logs are often the only way that anyone can see what happened.
Not having this information makes it extremely difficult to assess who did what when and to what extent access was gained.
Create and Follow a Logging Strategy
It's very rare that I see an application that has an actual logging strategy. Most of the time, we implement logging as an afterthought.
I guess that can be a strategy, but can we do better? I think we can.
When you add logging into the application, it's a good idea to have an overall consistent strategy. Use the same logging framework across all of the applications wherever possible.
This makes it easy to share configurations, such as message formats, and to adopt consistent logging patterns.
Consistency on when a message is a warning or an error and what logging levels to use also need to be documented.
When logging anything, the message format should always contain at a minimum the timestamp, current thread identifier, caller identity, and source code information.
All modern logging frameworks support this type of information out of the box.
Having all of this as part of your developer documentation would be a great way to create and maintain a consistent logging strategy across all of your business's applications.
Log the Complete Stack Trace
In many of the secure code reviews I’ve done, a mistake I commonly see is not logging the entire stack trace for an exception.
Take this hypothetical example, representative of the exact pattern I've seen many times in code reviews:
public Customer findCustomerByName(String customerName) {
try {
Customer c = customerService.findByName(customerName);
return c;
} catch (Exception ex) {
LOG.error("Exception looking up customer by name: " + ex.getMessage());
}
}
Now, there are a few things wrong with this example, but let’s just focus on handling the SQLException. Let’s say that in production you look at the logs and see this:
2018-03-02 09:29:47.287 ERROR 5166 --- [nio-8090-exec-1] com.scalyr.controllers.DemoController : org.hibernate.exception.SQLGrammarException: error executing work
That doesn’t tell you a whole lot. What caused the SQLGrammarException?
The logger classes all have an overload that takes a Throwable object and will handle constructing and writing out the stack trace.
By changing the code slightly, we can get a clearer picture of what's going on:
public Customer findCustomerByName(String customerName) {
try {
Customer c = customerService.findByName(customerName);
return c;
} catch (Exception ex) {
LOG.error("Exception looking up customer by name: " + ex.getMessage(), ex);
}
return null;
}
This code change we applied resulted in logging the full stack trace, which clearly shows some nefarious activity here (or fat fingers...).
2018-03-02 09:33:11.341 ERROR 5188 --- [nio-8090-exec-1] com.scalyr.controllers.DemoController : org.hibernate.exception.SQLGrammarException: error executing work org.hibernate.exception.SQLGrammarException: error executing work at org.hibernate.exception.internal.SQLExceptionTypeDelegate.convert(SQLExceptionTypeDelegate.java:63) ~[hibernate-core-5.0.12.Final.jar:5.0.12.Final] at org.hibernate.exception.internal.StandardSQLExceptionConverter.convert(StandardSQLExceptionConverter.java:42) ~[hibernate-core-5.0.12.Final.jar:5.0.12.Final] at org.hibernate.engine.jdbc.spi.SqlExceptionHelper.convert(SqlExceptionHelper.java:109) ~[hibernate-core-5.0.12.Final.jar:5.0.12.Final] ... omitted at org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61) [tomcat-embed-core-8.5.15.jar:8.5.15] at java.lang.Thread.run(Thread.java:748) [na:1.8.0_144] Caused by: java.sql.SQLSyntaxErrorException: malformed string: 'Acme'' at org.hsqldb.jdbc.JDBCUtil.sqlException(Unknown Source) ~[hsqldb-2.4.0.jar:2.4.0] at org.hsqldb.jdbc.JDBCUtil.sqlException(Unknown Source) ~[hsqldb-2.4.0.jar:2.4.0] at org.hsqldb.jdbc.JDBCStatement.fetchResult(Unknown Source) ~[hsqldb-2.4.0.jar:2.4.0] at org.hsqldb.jdbc.JDBCStatement.executeQuery(Unknown Source) ~[hsqldb-2.4.0.jar:2.4.0] at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[na:1.8.0_144] at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) ~[na:1.8.0_144] at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[na:1.8.0_144] at java.lang.reflect.Method.invoke(Method.java:498) ~[na:1.8.0_144] ... 105 common frames omitted Caused by: org.hsqldb.HsqlException: malformed string: 'Acme'' at org.hsqldb.error.Error.error(Unknown Source) ~[hsqldb-2.4.0.jar:2.4.0] at org.hsqldb.error.Error.error(Unknown Source) ~[hsqldb-2.4.0.jar:2.4.0] at org.hsqldb.ParserBase.read(Unknown Source) ~[hsqldb-2.4.0.jar:2.4.0] at org.hsqldb.ParserDQL.XreadPredicateRightPart(Unknown Source) ~[hsqldb-2.4.0.jar:2.4.0] at org.hsqldb.ParserDQL.XreadBooleanPrimaryOrNull(Unknown Source) ~[hsqldb-2.4.0.jar:2.4.0] ... 122 common frames omitted
Now if we were to see this in the logs, we can pretty immediately see what the issue is. Someone has attempted to look up a customer with the name of Acme’and it broke our SQL statement.
This exception is a clear indicator of a SQL injection and could be easily missed if someone analyzes the logs and only sees the original message.
They might not think much of it and move on to other issues, not catching a serious flaw.
Log All Java Exceptions
The “swallowing” of exceptions is another all-too-common issue I see.
An exception is thrown somewhere in the application and the developer has a catch block intending to handle the exception, but for some reason forgets to come back to it or decides that it isn't important.
The following example illustrates this problem:
public Customer findCustomerByName(String customerName) {
try {
Customer c = customerService.findByName(customerName);
return c;
} catch (Exception ex) {
// todo: Log using the new logging strategy..
}
return null;
}
This practice is all too common from my experience and definitely warrants being called out.
Logging the exception, rethrowing it, or just not handling it at all results in no indication in the logs that anything went wrong with the application.
There’s never a reason not to at least log an exception.
Swallowing exceptions like this results in hiding any problems with the underlying query or another abstraction, which may go unnoticed and may be the result of issues in business logic or a security flaw.
Don’t Return Exceptions to the User
When performing a security assessment of any kind, every piece of information you can learn about the application or its environment is potentially useful.
A seemingly innocuous error message may be just what a consultant (or an attacker) needs.
They could find the one exploit that may work against your system or greatly reduce the payloads needed to test for a SQL injection if an error message reveals something about the database system in use.
It’s also a common practice to simply return an exception message to the user through some kind of error handling.
I come across this a lot when testing authentication systems, as in the following screenshot:
The code that handles this might be doing something like this:
User findByUsername(String userName) throws UserNameNotFoundException {
EntityManager em = entityManagerFactory.createEntityManager();
return em.createQuery("from User where userName = :userName", User.class)
.setParameter("userName", userName)
.getSingleResult();
}
Later on, the exception is thrown and caught. The developer uses the exception message to construct an error that's passed along to the user. This results in the user being able to see the raw exception message.
public String login(Model model, String username, String password) {
try {
// attempt to login user
userService.login(username, password);
} catch (Exception ex) {
model.addAttribute("error", ex.getMessage());
}
return "login”;
Not only is this bad practice as far as exception handling goes, but it also opens up the application to user account validation.
Depending on the type of application you're working on, this could be a risk in itself.
Never return the contents of an exception object to the user. Catch the exception, log it, and return a generic response.
You never know what information the exception message may contain as code evolves, and the message itself may change in the future.
Don’t Log Sensitive Information
I mentioned that logs can be useful for not only debugging but also for compliance, audit, and forensics. Because logs have many uses and we have a tendency to just “log everything,” they can be an incredible source of information.
If logs contain usernames, passwords, session tokens, or other sensitive information, it really reduces the work for an attacker.
Logs will reveal the inner workings and failures of an application, all of which an attacker can use to attack the application further.
Due to this, we need to view and treat logs as sensitive and keep them secure. We probably already know not to log the following information:
- Credit card numbers
- Social security numbers
- Passwords
But the following types of information shouldn't be written to logs either:
- Session identifiers
- Authorization tokens
- Personal names
- Telephone numbers
- Information the user has opted out of (e.g., do not track)
There’s another issue: some jurisdictions don’t allow certain information to be tracked, and doing so violates the law.
Knowing the compliance requirements of the application and the data it processes is extremely important.
Don’t Be in the Dark
While logging isn't a complex task, there's a lot of subtlety and balance in getting it right. Too little information won't be very valuable. Too much information can be overwhelming if not relevant or isn't handled properly.
Application logging isn't optional. Without adequate logs, you're truly in the dark.
This post was written by Casey Dunham. Casey, who recently launched his own security business, is known for his unique approaches to all areas of application security, stemming from his 10+ year career as a professional software developer. His strengths include secure SDLC development consulting; threat modeling; developer training; and auditing web, mobile, and desktop applications for security flaws.
Top comments (0)