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

Padding Oracle Vulnerability #3

Closed
defuse opened this issue Mar 14, 2015 · 5 comments

Comments

@defuse
Copy link

commented Mar 14, 2015

This library does not authenticate ciphertexts, and there is a padding oracle vulnerability. I am procrastinating some math homework, so I'll write out a full description of the attack.

Suppose Mallory has an N-block ciphertext (IV, C1, C2, ..., CN) she wants to decrypt. Suppose also that she can choose ciphertexts to be decrypted, but all she can see is whether the substr() in unpad() returns false or not. This is a reasonable scenario, for example a web site might try to encrypt its cookies with this library to keep their contents secret from the user, and error out when decrypt() returns false, or might otherwise be distinguishable by treating false as the empty string.

Mallory can use this to find out the last byte of every plaintext block. Suppose Mallory wants to find the last byte of the plaintext block Pj corresponding to the ciphertext block Cj (call the IV C0).

For quick reference, here is the unpad function:

private function unpad($data)
{
        $pad = ord($data[strlen($data) - 1]);
        return substr($data, 0, -$pad);
}

If Mallory sends the ciphertext (C(j-1), Cj), i.e. the previous block as the IV, and the to-be-decrypted block as the only block in the ciphertext, then substr() will return false exactly when the last byte of Pj is greater than 16. We will call the two results either FALSE (substr() returned false) or SUCCESS (substr() returned a string). Here is how Mallory gets the last byte:

Input: C[j-1], C[j], byte arrays of length 16.
For X := 0 to 255:
    V := C[j-1]
    // XOR the last byte of plaintext with X
    V[15] := V[15] ^ X
    Send (V, C[j])
    // If XORing with X makes the last byte <= 16...
    If SUCCESS:
        // Check if the last byte is 16 by "adding one"
        V[15] := V[15] ^ 1
        Send (V, C[j])
        If FAIL:
            // The last byte now decrypts to 17.
            Output X ^ 1 ^ 17
        EndIf
    EndIf
EndFor

tl;dr: We alter the previous block until the last byte of the plaintext block is known to be 17. We know how it got changed to 17 (by XORing X^1), so undoing that (XORing 17 by X^1) gives Mallory the value of the last byte.

This attack highlights the danger of not authenticating the ciphertext, but in practice it is probably much easier to break, since the client will probably behave differently depending on the contents of the returned string, and thus Mallory would be able to decrypt entire blocks (it will depend on how this library gets used in the specific application).

Here is a working implementation of the attack: https://gist.github.com/defuse/9fbbec08dc792b0ca696

The fix is to authenticate your ciphertexts.

By the way, I am the author of the similarly-named php-encryption. I put a lot of time into making that one secure, and it has had a decent amount of review, so you may want to just replace this code with mine.

@defuse

This comment has been minimized.

@ondrejhlavacek

This comment has been minimized.

Copy link
Member

commented Mar 17, 2015

Hi, really appreciate your input. We'll look into it, fix it and let users know what happened.

@sarciszewski

This comment has been minimized.

Copy link

commented May 8, 2015

👍 I came here to say the same thing, @defuse

@Halama

This comment has been minimized.

Copy link
Member

commented May 11, 2015

Hi,
we are aware of this issue and we are going to fix it.
Meanwhile I've updated the readme https://github.com/keboola/php-encryption to mention this issue.

@Halama

This comment has been minimized.

Copy link
Member

commented May 12, 2015

After the discussion we decided that we will no longer support this library. Instead we will switch to php-encryption and this library is also suggested now in readme and on Packagist.
It doesn't make sense now to continue this library when the more mature library exists now.
This library is now marked as Abandoned on packagist https://packagist.org/packages/keboola/php-encryption
Thanks @defuse for your work.

@Halama Halama closed this May 12, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.