Friday, July 24, 2015

Have I Been Hacked?

Twenty-five years later I can still remember how I felt, returning home that day. Being burgled is a surrealistic experience: you notice a progression of little things that don't belong, and it may be quite a long time before your realize that your world has changed. For me, the first anomaly was that the front door of our two-family house was unlocked. OK, maybe my neighbor was out in the back yard. The second was that I could hear the music that I left on for the bird. As I walked up the stairway (unchanged), I saw that my front door was open. I didn't notice that the wood around the lock was splintered. I walked into what seemed a perfectly normal front room, and finally realized that something was wrong when I saw the VCR hanging by its antenna cable (+1 for tightening the cable with a wrench).

Now imagine a different scenario: the front door locked, the upper door standing open but unlocked, lights on that shouldn't be, and a few dirty dishes on the counter. All things that could be explained by me being particularly forgetful that morning. But still the sense that something was not quite right.

That was my feeling this week, as several of my friends reported receiving spam emails from me, warning me that my Yahoo account might have been hacked. The first was from my neighbor, and I discounted his report, thinking that the spammer might have hit another neighbor, gotten the neighborhood mailing list, and paired up random people. After all, the “From” address wasn't even mine! But then I got reports from friends that weren't neighbors, and even found a couple of the emails in my own GMail spam folder.

OK, so my Yahoo account got hacked, big deal. Maybe next time I'll be more careful with sharing passwords.

Except … the Yahoo account has its own password, one that's not saved anywhere but my head, so I have a hard time accepting that the account was actually broken into. And Yahoo's “activity report” claims that all logins in the past 30 days came from my home IP (a nice feature, it's one of the tabs on the profile page). And, I can still log into the account. I've never heard of anyone breaking into an account and just leaving it there, untouched.

And when I looked at the message, my email address wasn't to be found anywhere. It was “kdgregory,” but some server in a .cx domain. Different people reported different domains. Nor was a Yahoo server to be found in the headers. OK, headers can be forged, but I would have expected a forgery that at least attempted to look credible. According to the IP addresses in the headers, this email originated somewhere in India, went through a Japanese server, and then to its destination.

So I'm left with wondering what happened. Clearly these emails were based on information from my account, either headers from old messages (likely) or a contact list (less likely). But how? Googling for “yahoo data breach” turns up quite a few news stories, but nothing from this year. Did whoever acquired these addresses just sit on them for a year? And if yes, what other information did they glean from my account?

It's disquieting, this sense of perhaps being compromised. I think I would have been happier if they had changed my password and locked me out of the account. At least then I'd know I was being sloppy about security. As it is, I have no idea what (if anything) I did wrong. Or whether it will happen again.

Friday, July 17, 2015

The Achilles Heel of Try-With-Resources

Here's some code that you might see since Java 7 appeared. Can you spot the bug?

public List<String> extractData(File file, String encoding)
throws IOException
    List<String> results = new ArrayList<String>();
    try (BufferedReader rdr = new BufferedReader(new InputStreamReader(new FileInputStream(file), encoding)))
        // read each line and extract data
    return results;

OK, here's a hint: what if encoding is invalid?

The answer is that the InputStreamReader constructor will throw UnsupportedEncodingException (which is a subclass of IOException).

But this exception happens after the FileInputStream has been created, but before the body of the try-with-resources statement. Which means that the implicit finally in that statement won't get executed, and the file will remain open until the stream's finalizer executes.

To solve this problem, you need to break apart the constructor chain, to ensure that the actual resource is assigned to its own variable in the resource clause, and will therefore be closed. There are two ways to approach this: either use multiple resource clauses, or push non-resource constructors into the body of the statement.

In this case, the InputStreamReader and BufferedReader are simple decorator objects: if you neglect to close them, it doesn't change the behavior of the program. So we can construct them inside the body of the statement:

try (InputStream in = new FileInputStream(file))
    BufferedReader rdr = new BufferedReader(new InputStreamReader(in, encoding));
    // ...

Bug solved: regardless of what happens when constructing rdr, the underlying stream will be closed; we don't have to worry about a “too many open files” exception.

However, if we try to do something similar with output streams, we introduce a new bug:

try (OutputStream out = new FileOutputStream(file))
    // DON'T DO THIS!
    BufferedWriter wtr = new BufferedWriter(new OutputStreamWriter(out, encoding));
    // ...

The problem with this code is that decorators are important when writing. Unless you close the BufferedOutputStream, the data in its buffer won't be written to the file. On the positive side, you'll see this bug the first time you try to write a file. To solve it and still use the try-with-resources construct, you need to use multiple resource clauses:

try (OutputStream out = new FileOutputStream(file) ;
        OutputStreamWriter osw = new OutputStreamWriter(out, encoding) ;
        BufferedWriter wtr = new BufferedWriter(osw))
    // ...

There you have it, the only safe way to use try-with-resources. If you currently have code with nested constructors, I strongly recommend that you change it. And please change any public documentation that has nested constructors; explaining the bug to someone that is “just following the docs” wastes time that could be used for better things.