Improve array bounds checks in CipherState implementations

Thanks to Pietro Oliva for identifying these issues.
This commit is contained in:
Rhys Weatherley 2020-08-29 07:59:27 +10:00
parent a8dce061f6
commit 18e86b6f8b
5 changed files with 35 additions and 30 deletions

1
.gitignore vendored
View File

@ -3,3 +3,4 @@ target
.metadata
.project
doc
*.class

View File

@ -185,10 +185,11 @@ class AESGCMFallbackCipherState implements CipherState {
byte[] ciphertext, int ciphertextOffset, int length)
throws ShortBufferException {
int space;
if (ciphertextOffset > ciphertext.length)
space = 0;
else
space = ciphertext.length - ciphertextOffset;
if (ciphertextOffset < 0 || ciphertextOffset > ciphertext.length)
throw new IllegalArgumentException();
if (length < 0 || plaintextOffset < 0 || plaintextOffset > plaintext.length)
throw new IllegalArgumentException();
space = ciphertext.length - ciphertextOffset;
if (!haskey) {
// The key is not set yet - return the plaintext as-is.
if (length > space)
@ -214,16 +215,15 @@ class AESGCMFallbackCipherState implements CipherState {
int ciphertextOffset, byte[] plaintext, int plaintextOffset,
int length) throws ShortBufferException, BadPaddingException {
int space;
if (ciphertextOffset > ciphertext.length)
space = 0;
if (ciphertextOffset < 0 || ciphertextOffset > ciphertext.length)
throw new IllegalArgumentException();
else
space = ciphertext.length - ciphertextOffset;
if (length > space)
throw new ShortBufferException();
if (plaintextOffset > plaintext.length)
space = 0;
else
space = plaintext.length - plaintextOffset;
if (length < 0 || plaintextOffset < 0 || plaintextOffset > plaintext.length)
throw new IllegalArgumentException();
space = plaintext.length - plaintextOffset;
if (!haskey) {
// The key is not set yet - return the ciphertext as-is.
if (length > space)

View File

@ -218,10 +218,11 @@ class AESGCMOnCtrCipherState implements CipherState {
byte[] ciphertext, int ciphertextOffset, int length)
throws ShortBufferException {
int space;
if (ciphertextOffset > ciphertext.length)
space = 0;
else
space = ciphertext.length - ciphertextOffset;
if (ciphertextOffset < 0 || ciphertextOffset > ciphertext.length)
throw new IllegalArgumentException();
if (length < 0 || plaintextOffset < 0 || plaintextOffset > plaintext.length)
throw new IllegalArgumentException();
space = ciphertext.length - ciphertextOffset;
if (keySpec == null) {
// The key is not set yet - return the plaintext as-is.
if (length > space)
@ -262,16 +263,15 @@ class AESGCMOnCtrCipherState implements CipherState {
int ciphertextOffset, byte[] plaintext, int plaintextOffset,
int length) throws ShortBufferException, BadPaddingException {
int space;
if (ciphertextOffset > ciphertext.length)
space = 0;
if (ciphertextOffset < 0 || ciphertextOffset > ciphertext.length)
throw new IllegalArgumentException();
else
space = ciphertext.length - ciphertextOffset;
if (length > space)
throw new ShortBufferException();
if (plaintextOffset > plaintext.length)
space = 0;
else
space = plaintext.length - plaintextOffset;
if (length < 0 || plaintextOffset < 0 || plaintextOffset > plaintext.length)
throw new IllegalArgumentException();
space = plaintext.length - plaintextOffset;
if (keySpec == null) {
// The key is not set yet - return the ciphertext as-is.
if (length > space)

View File

@ -214,10 +214,11 @@ class ChaChaPolyCipherState implements CipherState {
public int encryptWithAd(byte[] ad, byte[] plaintext, int plaintextOffset,
byte[] ciphertext, int ciphertextOffset, int length) throws ShortBufferException {
int space;
if (ciphertextOffset > ciphertext.length)
space = 0;
else
space = ciphertext.length - ciphertextOffset;
if (ciphertextOffset < 0 || ciphertextOffset > ciphertext.length)
throw new IllegalArgumentException();
if (length < 0 || plaintextOffset < 0 || plaintextOffset > plaintext.length)
throw new IllegalArgumentException();
space = ciphertext.length - ciphertextOffset;
if (!haskey) {
// The key is not set yet - return the plaintext as-is.
if (length > space)
@ -241,16 +242,15 @@ class ChaChaPolyCipherState implements CipherState {
int ciphertextOffset, byte[] plaintext, int plaintextOffset,
int length) throws ShortBufferException, BadPaddingException {
int space;
if (ciphertextOffset > ciphertext.length)
space = 0;
if (ciphertextOffset < 0 || ciphertextOffset > ciphertext.length)
throw new IllegalArgumentException();
else
space = ciphertext.length - ciphertextOffset;
if (length > space)
throw new ShortBufferException();
if (plaintextOffset > plaintext.length)
space = 0;
else
space = plaintext.length - plaintextOffset;
if (length < 0 || plaintextOffset < 0 || plaintextOffset > plaintext.length)
throw new IllegalArgumentException();
space = plaintext.length - plaintextOffset;
if (!haskey) {
// The key is not set yet - return the ciphertext as-is.
if (length > space)

View File

@ -100,6 +100,8 @@ public interface CipherState extends Destroyable {
*
* @throws IllegalStateException The nonce has wrapped around.
*
* @throws IllegalArgumentException One of the parameters is out of range.
*
* The plaintext and ciphertext buffers can be the same for in-place
* encryption. In that case, plaintextOffset must be identical to
* ciphertextOffset.
@ -130,6 +132,8 @@ public interface CipherState extends Destroyable {
*
* @throws IllegalStateException The nonce has wrapped around.
*
* @throws IllegalArgumentException One of the parameters is out of range.
*
* The plaintext and ciphertext buffers can be the same for in-place
* decryption. In that case, ciphertextOffset must be identical to
* plaintextOffset.