[Openvpn-devel,S] Change in openvpn[master]: adjust_payload_max_cbc: Fix Coverity issue "Division or modulo by zero"

Message ID 4f6be5e2fc56379333283583d98e6ceb7fce3d3b-HTML@gerrit.openvpn.net
State Rejected
Headers show
Series [Openvpn-devel,S] Change in openvpn[master]: adjust_payload_max_cbc: Fix Coverity issue "Division or modulo by zero" | expand

Commit Message

flichtenheld (Code Review) Jan. 9, 2024, 11:09 a.m. UTC
Attention is currently required from: plaisthos.

Hello plaisthos,

I'd like you to do a code review.
Please visit

    http://gerrit.openvpn.net/c/openvpn/+/492?usp=email

to review the following change.


Change subject: adjust_payload_max_cbc: Fix Coverity issue "Division or modulo by zero"
......................................................................

adjust_payload_max_cbc: Fix Coverity issue "Division or modulo by zero"

In error cases cipher_kt_block_size can return 0.
This would lead to a divison by zero in round_down_uint.
By reordering the conditional we can make sure we only
take this path if we have a valid cipher to begin with.

Change-Id: Iaef904ee2448dc0b9ad396d3ad9ae21b9dd6281e
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
---
M src/openvpn/mss.c
1 file changed, 4 insertions(+), 7 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/92/492/1

Patch

diff --git a/src/openvpn/mss.c b/src/openvpn/mss.c
index 1566c64..797ebf7 100644
--- a/src/openvpn/mss.c
+++ b/src/openvpn/mss.c
@@ -210,13 +210,7 @@ 
 static inline size_t
 adjust_payload_max_cbc(const struct key_type *kt, size_t target)
 {
-    if (!cipher_kt_mode_cbc(kt->cipher))
-    {
-        /* With stream ciphers (or block cipher in stream modes like CFB, AEAD)
-         * we can just use the target as is */
-        return target;
-    }
-    else
+    if (cipher_kt_mode_cbc(kt->cipher))
     {
         /* With CBC we need at least one extra byte for padding and then need
          * to ensure that the resulting CBC ciphertext length, which is always
@@ -225,6 +219,9 @@ 
         target = round_down_size(target, block_size);
         return target - 1;
     }
+    /* With stream ciphers (or block cipher in stream modes like CFB, AEAD)
+     * we can just use the target as is */
+    return target;
 }
 
 static size_t