Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Decrypt - Malformed URI should throw an error and abort #4

Closed
suprafly opened this issue Apr 22, 2016 · 6 comments
Closed

Decrypt - Malformed URI should throw an error and abort #4

suprafly opened this issue Apr 22, 2016 · 6 comments

Comments

@suprafly
Copy link

When I encrypt some data, manually change it slightly and then attempt to decrypt that, Cipher breaks on an error:

** (ArgumentError) malformed URI "a%uisdfiuhdsiue889sdjkjk%2BXLAEFsgccTI4S9EApWiWpFpLcF7QPVoNiPiXMZz7g6dtjxi4ctfuAZCEqINevWvzeC4u%2FkbqELA4Zd0Shq9dPjhvP9oO3uGCfKGUIuOk30G1ohDN%2B5q%2BgjJD1c7JmN4klAKT%2B6S1PUWP41cp5M3xJgXcfbuJ5HbZVWvzHed%2FhM8vwGe7V6%2F4IPZGhJpa5Yo%2B8WgITql8%2FNWHLAa2MulQrZxJbuUY8h3ORQKMrE5eniVTLlLb9%2BPsyrolNlhyqkZCCbIc051jFJNmf8HDjebLraDG9ZdH71lqzu7M52aPw5WqdzZTz3kQ6c%2BfnGgjknkAgIAQHdPWBAlOoyt0S%2B6U7TEYHuF%2FsB1pgUPWNAzzlYmS1V%2B%2FZZgKJwihGHO5BR0luM4eayZCs7AgbBvQ%2FJ0jrrJKGEIRjcIr2%2FbY7H9qWn1RZqjqsdLEbkNg6h7y15xO0kSxSKPIuLLLpKVWHjVMIqXwo9nj55n9sskDeYjS2wiGI%2F5VnPGYsvuqPe9QgNuVat95%2F%2FAFQCqCabdemhA0KTcd9kIXMouCesw4mwQevXPfOu3K1aV4NNE2erPhUNSWsaHo4rfs19B%2FqEjDaRVKFCF4o8flOXb%2F5wzNapg8zpWjVhLXfuG4RBT4wV0jo9SOFksPALWiA7X9Q5M0MasmBtuoanMHATMLhMadLdKc7ugbVRseksfUXj2rkpyFgSM%2BA%3D%3D"
     stacktrace:
       (elixir) lib/uri.ex:238: URI.decode_www_form/1
       lib/cipher.ex:36: Cipher.decrypt/1

@suprafly
Copy link
Author

This one might be an error of the URI module itself: http://elixir-lang.org/docs/stable/elixir/URI.html#parse/1

Note this function expects a well-formed URI and does not perform any validation. See the examples section below of how URI.parse/1 can be used to parse a wide range of relative URIs.

@suprafly
Copy link
Author

suprafly commented Apr 22, 2016

You can catch the exception like this, then return a sensible error:

    try do
      Cipher.decrypt(string) do
    rescue
      e in ArgumentError -> {:error, e.message}
    end

@suprafly
Copy link
Author

In fact, that try rescue block will catch the other argument error I raised an issue about as well. Just need to return the correct msg.

@rubencaro
Copy link
Owner

I think I should not output the original error message when an internal procedure fails, such as this case. That is just an escaping/unescaping mechanism that Cipher uses internally. The crypted string is not meant to be an actual URL, so it would be meaningless to see an error message like "Malformed URI" at this stage. I fixed this by differenciating and documenting (hopefully) meaningful errors at the 2 stages of decrypting. So you would get {:error, "Could not decode string 'yourstring'"} or {:error, "Could not decrypt string 'yourstring'"}. See the docs for them:

    Returns `{:error, "Could not decode string 'yourstring'"}` if it failed in
    the first stage of decryption (unescaping and decoding given string). That
    means someone tampered your crypted data, or maybe the crypted string was
    not transferred properly.

    Returns `{:error, "Could not decrypt string 'yourstring'"}` if it failed in
    the last stage, the decryption itself. Usually means your decryption keys are
    not the same that were used to encrypt. But may also be some cases were a 
    tampered or wrongly transferred string can be actually unescaped and decoded
    successfully. They will fail in the decryption stage.

Many thanks!

@rubencaro
Copy link
Owner

I also added some of that to the README on master.

@rubencaro
Copy link
Owner

Just released 1.0.2 including this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants