diff --git a/src/org/thoughtcrime/securesms/crypto/MasterCipher.java b/src/org/thoughtcrime/securesms/crypto/MasterCipher.java index a606b9d2fa..cdc6263c8a 100644 --- a/src/org/thoughtcrime/securesms/crypto/MasterCipher.java +++ b/src/org/thoughtcrime/securesms/crypto/MasterCipher.java @@ -17,6 +17,7 @@ */ package org.thoughtcrime.securesms.crypto; +import android.support.annotation.NonNull; import android.util.Log; import org.thoughtcrime.securesms.util.Base64; @@ -92,7 +93,7 @@ public class MasterCipher { } } - public byte[] decryptBytes(byte[] decodedBody) throws InvalidMessageException { + public byte[] decryptBytes(@NonNull byte[] decodedBody) throws InvalidMessageException { try { Mac mac = getMac(masterSecret.getMacKey()); byte[] encryptedBody = verifyMacBody(mac, decodedBody); @@ -103,7 +104,7 @@ public class MasterCipher { return encrypted; } catch (GeneralSecurityException ge) { throw new InvalidMessageException(ge); - } + } } public byte[] encryptBytes(byte[] body) { @@ -153,7 +154,11 @@ public class MasterCipher { return Base64.encodeBytes(encryptedAndMacBody); } - private byte[] verifyMacBody(Mac hmac, byte[] encryptedAndMac) throws InvalidMessageException { + private byte[] verifyMacBody(@NonNull Mac hmac, @NonNull byte[] encryptedAndMac) throws InvalidMessageException { + if (encryptedAndMac.length < hmac.getMacLength()) { + throw new InvalidMessageException("length(encrypted body + MAC) < length(MAC)"); + } + byte[] encrypted = new byte[encryptedAndMac.length - hmac.getMacLength()]; System.arraycopy(encryptedAndMac, 0, encrypted, 0, encrypted.length); diff --git a/src/org/thoughtcrime/securesms/jobs/AttachmentDownloadJob.java b/src/org/thoughtcrime/securesms/jobs/AttachmentDownloadJob.java index 01020e8356..b54aaf1ff7 100644 --- a/src/org/thoughtcrime/securesms/jobs/AttachmentDownloadJob.java +++ b/src/org/thoughtcrime/securesms/jobs/AttachmentDownloadJob.java @@ -15,6 +15,7 @@ import org.thoughtcrime.securesms.jobs.requirements.MasterSecretRequirement; import org.thoughtcrime.securesms.jobs.requirements.MediaNetworkRequirement; import org.thoughtcrime.securesms.notifications.MessageNotifier; import org.thoughtcrime.securesms.util.Util; +import org.thoughtcrime.securesms.util.VisibleForTesting; import org.whispersystems.jobqueue.JobParameters; import org.whispersystems.jobqueue.requirements.NetworkRequirement; import org.whispersystems.libaxolotl.InvalidMessageException; @@ -122,10 +123,16 @@ public class AttachmentDownloadJob extends MasterSecretJob implements Injectable } } - private TextSecureAttachmentPointer createAttachmentPointer(MasterSecret masterSecret, PduPart part) + @VisibleForTesting + TextSecureAttachmentPointer createAttachmentPointer(MasterSecret masterSecret, PduPart part) throws InvalidPartException { - if (part.getContentLocation() == null) throw new InvalidPartException("null content location"); + if (part.getContentLocation() == null || part.getContentLocation().length == 0) { + throw new InvalidPartException("empty content id"); + } + if (part.getContentDisposition() == null || part.getContentDisposition().length == 0) { + throw new InvalidPartException("empty encrypted key"); + } try { AsymmetricMasterSecret asymmetricMasterSecret = MasterSecretUtil.getAsymmetricMasterSecret(context, masterSecret); @@ -164,7 +171,7 @@ public class AttachmentDownloadJob extends MasterSecretJob implements Injectable } } - private static class InvalidPartException extends Exception { + @VisibleForTesting static class InvalidPartException extends Exception { public InvalidPartException(String s) {super(s);} public InvalidPartException(Exception e) {super(e);} } diff --git a/test/unitTest/java/org/thoughtcrime/securesms/BaseUnitTest.java b/test/unitTest/java/org/thoughtcrime/securesms/BaseUnitTest.java index 7c356cefb9..030a969684 100644 --- a/test/unitTest/java/org/thoughtcrime/securesms/BaseUnitTest.java +++ b/test/unitTest/java/org/thoughtcrime/securesms/BaseUnitTest.java @@ -1,5 +1,6 @@ package org.thoughtcrime.securesms; +import android.content.Context; import android.os.Handler; import android.os.Looper; import android.util.Log; @@ -11,15 +12,25 @@ import org.mockito.stubbing.Answer; import org.powermock.api.mockito.PowerMockito; import org.powermock.core.classloader.annotations.PrepareForTest; import org.powermock.modules.junit4.PowerMockRunner; +import org.thoughtcrime.securesms.crypto.MasterSecret; + +import javax.crypto.spec.SecretKeySpec; import static org.mockito.Matchers.anyString; +import static org.powermock.api.mockito.PowerMockito.mock; import static org.powermock.api.mockito.PowerMockito.mockStatic; @RunWith(PowerMockRunner.class) -@PrepareForTest( { Log.class, Handler.class, Looper.class }) +@PrepareForTest({ Log.class, Handler.class, Looper.class }) public abstract class BaseUnitTest { + protected Context context; + protected MasterSecret masterSecret; + @Before public void setUp() throws Exception { + context = mock(Context.class); + masterSecret = new MasterSecret(new SecretKeySpec(new byte[16], "AES"), + new SecretKeySpec(new byte[16], "HmacSHA1")); mockStatic(Looper.class); mockStatic(Log.class); mockStatic(Handler.class); diff --git a/test/unitTest/java/org/thoughtcrime/securesms/crypto/MasterCipherTest.java b/test/unitTest/java/org/thoughtcrime/securesms/crypto/MasterCipherTest.java new file mode 100644 index 0000000000..8a75b1b29f --- /dev/null +++ b/test/unitTest/java/org/thoughtcrime/securesms/crypto/MasterCipherTest.java @@ -0,0 +1,24 @@ +package org.thoughtcrime.securesms.crypto; + +import org.junit.Before; +import org.junit.Test; +import org.powermock.core.classloader.annotations.PowerMockIgnore; +import org.thoughtcrime.securesms.BaseUnitTest; +import org.whispersystems.libaxolotl.InvalidMessageException; + +@PowerMockIgnore("javax.crypto.*") +public class MasterCipherTest extends BaseUnitTest { + private MasterCipher masterCipher; + + @Before + @Override + public void setUp() throws Exception { + super.setUp(); + masterCipher = new MasterCipher(masterSecret); + } + + @Test(expected = InvalidMessageException.class) + public void testEncryptBytesWithZeroBody() throws Exception { + masterCipher.decryptBytes(new byte[]{}); + } +} diff --git a/test/unitTest/java/org/thoughtcrime/securesms/jobs/AttachmentDownloadJobTest.java b/test/unitTest/java/org/thoughtcrime/securesms/jobs/AttachmentDownloadJobTest.java new file mode 100644 index 0000000000..5f040d78d4 --- /dev/null +++ b/test/unitTest/java/org/thoughtcrime/securesms/jobs/AttachmentDownloadJobTest.java @@ -0,0 +1,39 @@ +package org.thoughtcrime.securesms.jobs; + +import android.content.Context; + +import org.junit.Before; +import org.junit.Test; +import org.thoughtcrime.securesms.BaseUnitTest; +import org.thoughtcrime.securesms.database.PartDatabase.PartId; +import org.thoughtcrime.securesms.jobs.AttachmentDownloadJob.InvalidPartException; +import org.thoughtcrime.securesms.util.Util; + +import ws.com.google.android.mms.pdu.PduPart; + +import static org.powermock.api.mockito.PowerMockito.mock; + +public class AttachmentDownloadJobTest extends BaseUnitTest { + private AttachmentDownloadJob job; + + @Before + @Override + public void setUp() throws Exception { + super.setUp(); + job = new AttachmentDownloadJob(mock(Context.class), 1L, new PartId(1L, 1L)); + } + + @Test(expected = InvalidPartException.class) + public void testCreateAttachmentPointerInvalidId() throws Exception { + PduPart part = new PduPart(); + part.setContentDisposition(Util.toIsoBytes("a long and acceptable valid key like we all want")); + job.createAttachmentPointer(masterSecret, part); + } + + @Test(expected = InvalidPartException.class) + public void testCreateAttachmentPointerInvalidKey() throws Exception { + PduPart part = new PduPart(); + part.setContentDisposition(Util.toIsoBytes("1")); + job.createAttachmentPointer(masterSecret, part); + } +}