diff --git a/spec/lucky/cookies/cookie_jar_spec.cr b/spec/lucky/cookies/cookie_jar_spec.cr index 8de080f15..7a5521fde 100644 --- a/spec/lucky/cookies/cookie_jar_spec.cr +++ b/spec/lucky/cookies/cookie_jar_spec.cr @@ -84,9 +84,9 @@ describe Lucky::CookieJar do # meant to be a regression test to make sure we don't # accidentally break cookie decryption # - # this cookie was created with Lucky 0.27 + # this cookie was created with Lucky 1.5 cookie_key = "cookie_key" - cookie_value = "bHVja3k=--hY71kbRfob4pb9NS7wJpWKOBRhF+kwYPsHRQQanyXzGSKsCO6MIHCZfRBxDRqqm6" + cookie_value = "bHVja3k=--WyJuaEd0U1poaVRPeUdkTlRSMVVKSURBTUc2bGgyN1l1d3RUZE1rZnR4OVNaVG1vbmNkUU9UT1ZlNzZzWmoySlhjIiwiMHFLa3ZFS1RBMFRMaTZsTFZwT1Z3NFNGMUVzPSJd" cookies = HTTP::Cookies.new cookies[cookie_key] = cookie_value jar = Lucky::CookieJar.from_request_cookies(cookies) @@ -94,6 +94,19 @@ describe Lucky::CookieJar do JSON.parse(jar.get(cookie_key)).should eq({"key" => "value", "abc" => "123"}) end + it "returns nil when a valid cookie from a former Lucky version is decrypted" do + # Previous versions of Lucky had a vulnerability + # and the encrypted cookie can't be decrypted + # + # this cookie was created with Lucky 0.27 + cookie_key = "cookie_key" + cookie_value = "bHVja3k=--hY71kbRfob4pb9NS7wJpWKOBRhF+kwYPsHRQQanyXzGSKsCO6MIHCZfRBxDRqqm6" + cookies = HTTP::Cookies.new + cookies[cookie_key] = cookie_value + jar = Lucky::CookieJar.from_request_cookies(cookies) + jar.get?(cookie_key).should eq(nil) + end + describe "#set" do it "only sets the name, http_only, and value if no 'on_set' block is set" do Lucky::CookieJar.temp_config(on_set: nil) do diff --git a/src/lucky/cookies/cookie_jar.cr b/src/lucky/cookies/cookie_jar.cr index 41acaed58..fb6c709fb 100644 --- a/src/lucky/cookies/cookie_jar.cr +++ b/src/lucky/cookies/cookie_jar.cr @@ -138,11 +138,11 @@ class Lucky::CookieJar end private def encrypt(raw_value : String) : String - encrypted = encryptor.encrypt(raw_value) + encrypted = encryptor.encrypt_and_sign(raw_value) String.build do |value| value << LUCKY_ENCRYPTION_PREFIX - value << Base64.strict_encode(encrypted) + value << encrypted end end @@ -150,8 +150,7 @@ class Lucky::CookieJar return unless encrypted_with_lucky?(cookie_value) base_64_encrypted_part = cookie_value.lchop(LUCKY_ENCRYPTION_PREFIX) - decoded = Base64.decode(base_64_encrypted_part) - String.new(encryptor.decrypt(decoded)) + String.new(encryptor.verify_and_decrypt(base_64_encrypted_part)) rescue e # an error happened while decrypting the cookie # we will treat that as if no cookie was passed diff --git a/src/lucky/support/message_encryptor.cr b/src/lucky/support/message_encryptor.cr index 98375fea4..74a03c7a4 100644 --- a/src/lucky/support/message_encryptor.cr +++ b/src/lucky/support/message_encryptor.cr @@ -12,7 +12,7 @@ module Lucky end # Encrypt and sign a message. We need to sign the message in order to avoid - # padding attacks. Reference: http://www.limited-entropy.com/padding-oracle-attacks. + # padding attacks. Reference: https://en.wikipedia.org/wiki/Padding_oracle_attack. def encrypt_and_sign(value : Slice(UInt8)) : String verifier.generate(encrypt(value)) end @@ -22,7 +22,7 @@ module Lucky end # Verify and Decrypt a message. We need to verify the message in order to - # avoid padding attacks. Reference: http://www.limited-entropy.com/padding-oracle-attacks. + # avoid padding attacks. Reference: https://en.wikipedia.org/wiki/Padding_oracle_attack. def verify_and_decrypt(value : String) : Bytes decrypt(verifier.verify_raw(value)) end