From 7bdc856f0d0401f848343f5e55b8b45364e02a4f Mon Sep 17 00:00:00 2001
From: Arne Welzel <arne.welzel@corelight.com>
Date: Wed, 11 Dec 2024 13:04:45 +0100
Subject: [PATCH] QUIC/decrypt_crypto: Actually check if decryption was
 successful

...and bail if it wasn't.

PCAP was produced using OSS-Fuzz input from issue 383379789.
---
 src/analyzer/protocol/quic/decrypt_crypto.cc    |   5 +++--
 .../analyzer.log.cut                            |   3 +++
 .../conn.log.cut                                |   5 +++++
 .../Traces/quic/383379789-decrypt-crash.pcap    | Bin 0 -> 43734 bytes
 .../base/protocols/quic/decrypt-crash.zeek      |   8 ++++++++
 5 files changed, 19 insertions(+), 2 deletions(-)
 create mode 100644 testing/btest/Baseline/scripts.base.protocols.quic.decrypt-crash/analyzer.log.cut
 create mode 100644 testing/btest/Baseline/scripts.base.protocols.quic.decrypt-crash/conn.log.cut
 create mode 100644 testing/btest/Traces/quic/383379789-decrypt-crash.pcap
 create mode 100644 testing/btest/scripts/base/protocols/quic/decrypt-crash.zeek

diff --git a/src/analyzer/protocol/quic/decrypt_crypto.cc b/src/analyzer/protocol/quic/decrypt_crypto.cc
index efe83c5343..fa496413ae 100644
--- a/src/analyzer/protocol/quic/decrypt_crypto.cc
+++ b/src/analyzer/protocol/quic/decrypt_crypto.cc
@@ -152,7 +152,7 @@ Function that calls the AEAD decryption routine, and returns the decrypted data.
 */
 hilti::rt::Bytes decrypt(const std::vector<uint8_t>& client_key, const hilti::rt::Bytes& all_data,
                          uint64_t payload_length, const DecryptionInformation& decryptInfo) {
-    int out, out2, res;
+    int out, out2;
 
     if ( payload_length < decryptInfo.packet_number_length + AEAD_TAG_LENGTH )
         throw hilti::rt::RuntimeError(hilti::rt::fmt("payload too small %ld < %ld", payload_length,
@@ -202,7 +202,8 @@ hilti::rt::Bytes decrypt(const std::vector<uint8_t>& client_key, const hilti::rt
     EVP_CipherUpdate(ctx, decrypt_buffer.data(), &out, encrypted_payload, encrypted_payload_size);
 
     // Validate whether the decryption was successful or not
-    EVP_CipherFinal_ex(ctx, NULL, &out2);
+    if ( EVP_CipherFinal_ex(ctx, NULL, &out2) == 0 )
+        throw hilti::rt::RuntimeError("decryption failed");
 
     // Copy the decrypted data from the decrypted buffer into a Bytes instance.
     return hilti::rt::Bytes(decrypt_buffer.data(), decrypt_buffer.data() + out);
diff --git a/testing/btest/Baseline/scripts.base.protocols.quic.decrypt-crash/analyzer.log.cut b/testing/btest/Baseline/scripts.base.protocols.quic.decrypt-crash/analyzer.log.cut
new file mode 100644
index 0000000000..1a3b7336c7
--- /dev/null
+++ b/testing/btest/Baseline/scripts.base.protocols.quic.decrypt-crash/analyzer.log.cut
@@ -0,0 +1,3 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+ts	uid	cause	analyzer_kind	analyzer_name	failure_reason
+1733909795.417573	C4J4Th3PJpwUYZZ6gc	violation	protocol	QUIC	decryption failed (<...>/QUIC.spicy:<location>)
diff --git a/testing/btest/Baseline/scripts.base.protocols.quic.decrypt-crash/conn.log.cut b/testing/btest/Baseline/scripts.base.protocols.quic.decrypt-crash/conn.log.cut
new file mode 100644
index 0000000000..e9151b0dee
--- /dev/null
+++ b/testing/btest/Baseline/scripts.base.protocols.quic.decrypt-crash/conn.log.cut
@@ -0,0 +1,5 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+ts	uid	proto	history	service
+XXXXXXXXXX.XXXXXX	ClEkJM2Vm5giqnMf4h	udp	D	-
+XXXXXXXXXX.XXXXXX	C4J4Th3PJpwUYZZ6gc	udp	D	-
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	tcp	F	ftp
diff --git a/testing/btest/Traces/quic/383379789-decrypt-crash.pcap b/testing/btest/Traces/quic/383379789-decrypt-crash.pcap
new file mode 100644
index 0000000000000000000000000000000000000000..aeb980850817a541f4956db12eada9abb9381664
GIT binary patch
literal 43734
zcmeI*ziU%b6u|NG()3}Taqt%oE~3$tLUU1ZQ~W_H1%vpbA`+xJB&86dqDvmsW+)=~
z4-}jf9CT1{Dmdz*lcJl5OI-?1QluSP&%KTGXB4H@yu9y$`{=v5x$m4$r<9(X4=<lT
zNJvr^!V^z#c3-|U_^6hayBEa`_j<xzy$#aaA#z3UD!!b&P$&*n=X&f*+eNy24(|Ib
z#di<i?-Vnv(Jhas#%6<{G8C31%vVj;<cu-7O}A4fpEKDTuV;pJYL_V#b=sI9xTz~s
z;hc?+BAs$It6PUQsfs^3YM(<OfB*srAb<b@2q1s}0tiG^z<t{LKbNSIkv9SeAb<b@
z2q1s}0tmF4z-+4}L_TXEa9|DgU<(8gKp-vx-y474i)6jWlJo!j63OIA8UX|lKmY**
z5I_I{1Q0+V!U8AOF(S-F>Ifi!00IagfB*srAb<b@2q1s}0tg_000IagfB*srw5z~O
zyB0=W2p|v>f&DS5lQI!N0D+|fd+fQqd6A<!ZHJ4-xx9Bi^;hNG`9AN=>3cuJlU}d?
z)avv4Uc23wvmJ6+f40NL!ZE)YkuII8Z?=a5kNA4(uZ&N6FYH;oE5q)!cfgltN5l=c
zV)ea7MC@ksdxhm!y3~_oV#3RK<7JVwed`vPv?mcKCJMLo&Mya@(^EkZJRQ5XF*|tN
zt{@OVAie_I-p03fDn|eT1Q0*~0R#|0009ILKp+MJUt>@sB_e<T0tg_000IagfB*sr
zAb<b@2q1s}0tg_000IagfB*u~65xAvqV@YCTLch5009ILKmdU@5wPvsrAkT$v>rQL
z{I;|FiR(CT)?wakUiWs5?6q|B&hxr1>Jr;|zNA~+rM2vmX*9)O*Lgc!{7vWi^f&#I
zQhj2r)OY{QqE7n7o#)HD>-Ayw`M>O<uj`^esKs{CAKEHz{L@<8=3Ed!009ILXmx=d
Yt)3wHA%Fk^2q1s}0tg_0z<LY(0M_(m9smFU

literal 0
HcmV?d00001

diff --git a/testing/btest/scripts/base/protocols/quic/decrypt-crash.zeek b/testing/btest/scripts/base/protocols/quic/decrypt-crash.zeek
new file mode 100644
index 0000000000..4b64b04dc4
--- /dev/null
+++ b/testing/btest/scripts/base/protocols/quic/decrypt-crash.zeek
@@ -0,0 +1,8 @@
+# @TEST-DOC: Trace produced by OSS-Fuzz triggered a crash due to using a too small local buffer for decryption.
+
+# @TEST-REQUIRES: ${SCRIPTS}/have-spicy
+# @TEST-EXEC: zeek -Cr $TRACES/quic/383379789-decrypt-crash.pcap base/protocols/quic %INPUT
+# @TEST-EXEC: zeek-cut -m ts uid proto history service < conn.log > conn.log.cut
+# @TEST-EXEC: zeek-cut -m ts uid cause analyzer_kind analyzer_name failure_reason < analyzer.log > analyzer.log.cut
+# @TEST-EXEC: btest-diff conn.log.cut
+# @TEST-EXEC: TEST_DIFF_CANONIFIER='sed -E "s/\((.+)\.spicy:[0-9]+:[0-9]+(-[0-9]+:[0-9]+)?\)/(\1.spicy:<location>)/g" | $SCRIPTS/diff-remove-abspath' btest-diff analyzer.log.cut