From bb31226e091da3738b6fcfa80f572e4a3163f738 Mon Sep 17 00:00:00 2001
From: Andrew Steinborn <git@steinborn.me>
Date: Wed, 16 Sep 2020 00:07:52 -0400
Subject: [PATCH] Clean up more cipher code and make it -Wall + -Werror clean

---
 native/compile-linux.sh              | 12 +++++++++---
 native/src/main/c/jni_cipher.c       | 19 +++++++++++--------
 native/src/main/c/jni_zlib_deflate.c |  3 +--
 native/src/main/c/jni_zlib_inflate.c |  4 ++++
 4 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/native/compile-linux.sh b/native/compile-linux.sh
index 6d92b6462..40744df9b 100755
--- a/native/compile-linux.sh
+++ b/native/compile-linux.sh
@@ -1,5 +1,11 @@
 #!/bin/bash
 
+if [ ! "$CC" ]; then
+  # The libdeflate authors recommend that we build using GCC as it produces "slightly faster binaries":
+  # https://github.com/ebiggers/libdeflate#for-unix
+  export CC=gcc
+fi
+
 if [ ! -d libdeflate ]; then
   echo "Cloning libdeflate..."
   git clone https://github.com/ebiggers/libdeflate.git
@@ -10,10 +16,10 @@ cd libdeflate || exit
 CFLAGS="-fPIC -O2" make
 cd ..
 
-CFLAGS="-O3 -I$JAVA_HOME/include/ -I$JAVA_HOME/include/linux/ -fPIC -shared -Wl,-z,noexecstack"
+CFLAGS="-O2 -I$JAVA_HOME/include/ -I$JAVA_HOME/include/linux/ -fPIC -shared -Wl,-z,noexecstack -Wall -Werror"
 ARCH=$(uname -m)
 mkdir -p src/main/resources/linux_$ARCH
-gcc $CFLAGS -Ilibdeflate src/main/c/jni_util.c src/main/c/jni_zlib_deflate.c src/main/c/jni_zlib_inflate.c \
+$CC $CFLAGS -Ilibdeflate src/main/c/jni_util.c src/main/c/jni_zlib_deflate.c src/main/c/jni_zlib_inflate.c \
     libdeflate/libdeflate.a -o src/main/resources/linux_$ARCH/velocity-compress.so
-gcc $CFLAGS -I $MBEDTLS_ROOT/include -shared src/main/c/jni_util.c src/main/c/jni_cipher.c \
+$CC $CFLAGS -shared src/main/c/jni_util.c src/main/c/jni_cipher.c \
     -o src/main/resources/linux_$ARCH/velocity-cipher.so -lcrypto
\ No newline at end of file
diff --git a/native/src/main/c/jni_cipher.c b/native/src/main/c/jni_cipher.c
index 1872865b7..83515be52 100644
--- a/native/src/main/c/jni_cipher.c
+++ b/native/src/main/c/jni_cipher.c
@@ -12,25 +12,28 @@ Java_com_velocitypowered_natives_encryption_OpenSslCipherImpl_init(JNIEnv *env,
     jbyteArray key,
     jboolean encrypt)
 {
+    jsize keyLen = (*env)->GetArrayLength(env, key);
+    if (keyLen != 16) {
+        throwException(env, "java/lang/IllegalArgumentException", "cipher not 16 bytes");
+        return 0;
+    }
+
     EVP_CIPHER_CTX *ctx = EVP_CIPHER_CTX_new();
     if (ctx == NULL) {
         throwException(env, "java/lang/OutOfMemoryError", "allocate cipher");
         return 0;
     }
 
-    jsize keyLen = (*env)->GetArrayLength(env, key);
-    jbyte* keyBytes = (*env)->GetPrimitiveArrayCritical(env, key, NULL);
-    if (keyBytes == NULL) {
-        EVP_CIPHER_CTX_free(ctx);
-        throwException(env, "java/lang/OutOfMemoryError", "cipher get key");
+    // Since we know the array size is always bounded, we can just use Get<Primitive>ArrayRegion
+    // and save ourselves some error-checking headaches.
+    jbyte keyBytes[16];
+    (*env)->GetByteArrayRegion(env, key, 0, keyLen, (jbyte*) keyBytes);
+    if ((*env)->ExceptionCheck(env)) {
         return 0;
     }
 
     int result = EVP_CipherInit(ctx, EVP_aes_128_cfb8(), (byte*) keyBytes, (byte*) keyBytes,
         encrypt);
-    // Release the key byte array now - we won't need it
-    (*env)->ReleasePrimitiveArrayCritical(env, key, keyBytes, 0);
-
     if (result != 1) {
         EVP_CIPHER_CTX_free(ctx);
         throwException(env, "java/security/GeneralSecurityException", "openssl initialize cipher");
diff --git a/native/src/main/c/jni_zlib_deflate.c b/native/src/main/c/jni_zlib_deflate.c
index 9a954081c..8dcd60f09 100644
--- a/native/src/main/c/jni_zlib_deflate.c
+++ b/native/src/main/c/jni_zlib_deflate.c
@@ -34,8 +34,7 @@ Java_com_velocitypowered_natives_compression_NativeZlibDeflate_process(JNIEnv *e
     jlong sourceAddress,
     jint sourceLength,
     jlong destinationAddress,
-    jint destinationLength,
-    jboolean finish)
+    jint destinationLength)
 {
     struct libdeflate_compressor *compressor = (struct libdeflate_compressor *) ctx;
     size_t produced = libdeflate_zlib_compress(compressor, (void *) sourceAddress, sourceLength,
diff --git a/native/src/main/c/jni_zlib_inflate.c b/native/src/main/c/jni_zlib_inflate.c
index d6a0c09df..d91319089 100644
--- a/native/src/main/c/jni_zlib_inflate.c
+++ b/native/src/main/c/jni_zlib_inflate.c
@@ -53,5 +53,9 @@ Java_com_velocitypowered_natives_compression_NativeZlibInflate_process(JNIEnv *e
             // These cases are the same for us. We expect the full uncompressed size to be known.
             throwException(env, "java/util/zip/DataFormatException", "uncompressed size is inaccurate");
             return JNI_FALSE;
+        default:
+            // Unhandled case
+            throwException(env, "java/util/zip/DataFormatException", "unknown libdeflate return code");
+            return JNI_FALSE;
     }
 }
\ No newline at end of file