diff mbox series

[Openvpn-devel,S] Change in openvpn[release/2.6]: NTLM: add length check to add_security_buffer

Message ID 24b0de3b3035d04c48fef070956d4e3bf7d03131-HTML@gerrit.openvpn.net
State Superseded
Headers show
Series [Openvpn-devel,S] Change in openvpn[release/2.6]: NTLM: add length check to add_security_buffer | expand

Commit Message

flichtenheld (Code Review) Jan. 10, 2024, 2:57 p.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/+/493?usp=email

to review the following change.


Change subject: NTLM: add length check to add_security_buffer
......................................................................

NTLM: add length check to add_security_buffer

Especially ntlmv2_response can be very big, so make sure
we not do exceed the size of the phase3 buffer.

Change-Id: Icea931d29e3e504e23e045539b21013b42172664
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
---
M src/openvpn/ntlm.c
1 file changed, 10 insertions(+), 5 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/93/493/1
diff mbox series

Patch

diff --git a/src/openvpn/ntlm.c b/src/openvpn/ntlm.c
index 2e77214..2b735ec 100644
--- a/src/openvpn/ntlm.c
+++ b/src/openvpn/ntlm.c
@@ -167,8 +167,13 @@ 
 
 static void
 add_security_buffer(int sb_offset, void *data, int length,
-                    unsigned char *msg_buf, int *msg_bufpos)
+                    unsigned char *msg_buf, int *msg_bufpos, size_t msg_bufsize)
 {
+    if (*msg_bufpos + length > msg_bufsize)
+    {
+        msg(M_WARN, "NTLM: security buffer too big for message buffer");
+        return;
+    }
     /* Adds security buffer data to a message and sets security buffer's
      * offset and length */
     msg_buf[sb_offset] = (unsigned char)length;
@@ -396,20 +401,20 @@ 
     if (ntlmv2_enabled)      /* NTLMv2 response */
     {
         add_security_buffer(0x14, ntlmv2_response, ntlmv2_blob_size + 16,
-                            phase3, &phase3_bufpos);
+                            phase3, &phase3_bufpos, sizeof(phase3));
     }
     else       /* NTLM response */
     {
-        add_security_buffer(0x14, ntlm_response, 24, phase3, &phase3_bufpos);
+        add_security_buffer(0x14, ntlm_response, 24, phase3, &phase3_bufpos, sizeof(phase3));
     }
 
     /* username in ascii */
     add_security_buffer(0x24, username, strlen(username), phase3,
-                        &phase3_bufpos);
+                        &phase3_bufpos, sizeof(phase3));
 
     /* Set domain. If <domain> is empty, default domain will be used
      * (i.e. proxy's domain) */
-    add_security_buffer(0x1c, domain, strlen(domain), phase3, &phase3_bufpos);
+    add_security_buffer(0x1c, domain, strlen(domain), phase3, &phase3_bufpos, sizeof(phase3));
 
     /* other security buffers will be empty */
     phase3[0x10] = phase3_bufpos;     /* lm not used */