Problematic with unhandled thrown error ...

[ChrisGitIt]

session-file-store/lib/session-file-helpers.js

Line 179 in b89a42b

data = options.decoder(helpers.decrypt(options, data, sessionId));

Hi all,

i just had a look at this lib in order to understand session handling.

I'm not fluently understanding the code, but it looks like there is an unhandled thrown error which might bring down apps that are using this lib.

Here is the function that is supposed to handle a decrypt:

data = options.decoder(helpers.decrypt(options, data, sessionId));

And this is the function call itself that throws an error that seems to be unhandled.

decrypt: function (options, data, sessionId) {
    var plaintext = null;

    options.kruptein.get(options.secret, data, function(err, pt) {
      if (err)
        throw err;

      plaintext = pt;
    });
    
    return plaintext;
  }

[ChrisGitIt]

Bye the way, the decrypt call to "options.kruptein.get()" seem to be async (due to usage of a callback).

But the call is not async and therefore this is working at all.
BUT IF the "options.kruptein.get()" would be async (with any update), the "decrypt" function itself would return plaintext = null.

[jas-]

When you manually inspect the session files are you not seeing cipher text values?

The session object would fail completely if 'null' was being turned into ciphertext as you suggest.

I think a better question would be if either creation or decryption of a ciphertext value occurs do you want to have this fail open vs. its current fail closed methods?

Assuming the security of the session data is to be protected, throwing an error vs. continuing would be beneficial to the protection of the data stored.

[ChrisGitIt]

@jas- Haven't further inspect the session object. I suppose its working as long as no error is thrown.

I think a better question would be if either creation or decryption of a ciphertext value occurs do you want to have this fail open vs. its current fail closed methods? Assuming the security of the session data is to be protected, throwing an error vs. continuing would be beneficial to the protection of the data stored.

I don't get it ... there is a callback function (

session-file-store/lib/session-file-helpers.js

Line 161 in b89a42b

get: function (sessionId, options, callback) {

) that accepts an error as first argument. I think i'm expecting a proper error handling by just using this callback and passing the error to.