• Post Reply Bookmark Topic Watch Topic
  • New Topic
programming forums Java Mobile Certification Databases Caching Books Engineering Micro Controllers OS Languages Paradigms IDEs Build Tools Frameworks Application Servers Open Source This Site Careers Other Pie Elite all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Tim Cooke
  • paul wheaton
  • Jeanne Boyarsky
  • Ron McLeod
Sheriffs:
  • Paul Clapham
  • Liutauras Vilda
  • Devaka Cooray
Saloon Keepers:
  • Tim Holloway
  • Roland Mueller
Bartenders:

Security Vulnerability: Throwing an Exception from a constructor

 
Ranch Hand
Posts: 590
Eclipse IDE Chrome Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
When throwing an Exception from a constructor of a class in Java you are leaving yourself open to a security vulnerability.

See here https://www.securecoding.cert.org/confluence/display/java/OBJ11-J.+Be+wary+of+letting+constructors+throw+exceptions

Recently I have started to using https://docs.oracle.com/javase/7/docs/api/java/util/Objects.html#requireNonNull(T,%20java.lang.String) in methods and constructors to validate for null i.e.



If the argument to the constructor is null then a NullPointerException is thrown from the constructor. So we have arrived in the situation where there is a known security vulnerability.

The article above mentions a strategy for getting around this security flaw (ensuring the Exception is thrown before the constructor of Object is finished executing).

To implement this strategy for every constructor where I want to use requireNonNull I think would be a serious overkill. So I’m wondering what I should do?

1. Don’t ever use requireNonNull to validate for non-null in the constructor
2. Use requireNonNull in the constructor in conjunction with the strategy to avoid the security vulnerability
3. Only use the strategy to avoid the security vulnerability in particular classes (not sure what the criteria would be to determine which classes), and for every other class simply use requireNonNull without this strategy

Any thoughts?
 
Rancher
Posts: 4801
50
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
The easiest solution is to simply make your class final.

I see that's the first solution in there.

Even though I don't write code that escapes the company I'm writing it for I still tend to make classes final.
 
Bartender
Posts: 15737
368
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Do you write classes that have to worry about these kinds of things?

The class must be used in a multi-threaded environment, and it must be able to perform dangerous operations when in an inconsistent state. One example is when the exception causes the object to escape without a security manager.

The bottom line is, don't worry about this. If you don't know whether this issue relates to your code, it probably doesn't. Write normal clear Java, and please use exceptions to do parameter checking in your constructor.
 
Sean Keane
Ranch Hand
Posts: 590
Eclipse IDE Chrome Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Good point. I can make some of my classes final. There's always the chance that another developer in future could remove final to subclass, unaware of why it was final. I guess I could document why it's final.

As to whether I actually need to worry about this vulnerability. Good question...

My code is running in a multithreaded environment.

Some of my classes would have direct access to the production database. So I guess these would be deemed vulnerable?

"please use exceptions to do parameter checking in your constructor." Using Objects.requireNonNull is using exceptions to do parameter checking?
 
Dave Tolls
Rancher
Posts: 4801
50
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
You shouldn't have to document why it's final.
All classes should be final (IMO) unless someone has to extend them.

The only place you need to worry about this sort of security issue (again IMO) is for code that is deployed outside of your servers.  SO apps deployed to desktops or external servers.
If this is a web app, for example, this is a completely unnecessary concern.
 
Stephan van Hulst
Bartender
Posts: 15737
368
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
I agree with Dave. I don't think there's any reason for concern.

Sean Keane wrote:Using Objects.requireNonNull is using exceptions to do parameter checking?


Yes. Objects.requireNonNull() throws a NullPointerException if the argument is null.
 
Sean Keane
Ranch Hand
Posts: 590
Eclipse IDE Chrome Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Dave Tolls wrote:All classes should be final (IMO) unless someone has to extend them..



Ideally yes, but then it makes unit testing more difficult. Requiring you to use something like Powermock to mock the final class - I'd rather not use this.
 
Dave Tolls
Rancher
Posts: 4801
50
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Sean Keane wrote:
Ideally yes, but then it makes unit testing more difficult. Requiring you to use something like Powermock to mock the final class - I'd rather not use this.



This is why business classes (ie the sort you would actually unit test) should be based on interfaces.

In any case, is this a system that you actually need to be concerned about this at all?
 
Sean Keane
Ranch Hand
Posts: 590
Eclipse IDE Chrome Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
I think from the feedback here and some more thinking that I don't need to worry about this security vulnerability in the context I am throwing exceptions in the constructor.

In some cases I have interfaces so the implementation can be final. But like most things in life it's a trade off; I can't have a proliferation of interfaces purely for sake of marking implementation final.
 
Stephan van Hulst
Bartender
Posts: 15737
368
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Sean Keane wrote:I can't have a proliferation of interfaces purely for sake of marking implementation final.


Why not? Interfaces are good.
 
Sean Keane
Ranch Hand
Posts: 590
Eclipse IDE Chrome Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Technically nothing stopping me of course. Does every single class you want to unit test in applications you develop implement an interface?
 
Dave Tolls
Rancher
Posts: 4801
50
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Sean Keane wrote:Technically nothing stopping me of course. Does every single class you want to unit test in applications you develop implement an interface?



It's not about unit testing.
It's about writing a contract (the interface) and later writing the implementation(s) for that contract.

It just has a nice side effect in that it makes unit testing a lot easier.

Just looking at the code I have in front of me and every service is an interface first and an implementation second.
I'm hard pushed to think of an app I've written in the past decade that hasn't been like that, though that could just be rotten memory.
 
Sean Keane
Ranch Hand
Posts: 590
Eclipse IDE Chrome Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
I agree it's good to have interfaces. From experience though, with significantly large applications it will not be a case that every class you want to unit test will have an interface.

Anyway. I think we are getting slightly off topic . So in summary, for this particular vulnerability, if we make the class final we are safe and if we still want to be able to mock the class for testing just ensure it has an interface to describe it's behaviour.
 
If somebody says you look familiar, tell them you are in porn. Or in these tiny ads:
New web page for Paul's Rocket Mass Heaters movies
https://coderanch.com/t/785239/web-page-Paul-Rocket-Mass
reply
    Bookmark Topic Watch Topic
  • New Topic