diff mbox series

[Openvpn-devel,S] Change in openvpn[master]: Allow trailing \r and \n in control channel message

Message ID ba2a0b29ea6861e15209726459371e7c8fee990e-HTML@gerrit.openvpn.net
State New
Headers show
Series [Openvpn-devel,S] Change in openvpn[master]: Allow trailing \r and \n in control channel message | expand

Commit Message

ralf_lici (Code Review) July 8, 2024, 10:09 a.m. UTC 
Attention is currently required from: flichtenheld.

Hello flichtenheld,

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

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

to review the following change.


Change subject: Allow trailing \r and \n in control channel message
......................................................................

Allow trailing \r and \n in control channel message

Writing a reason from a script will easily end up adding extra \r\n characters
at the end of the reason. Our current code pushes this to the peer. So be more
liberal in accepting these message.

Closes openvpn/openvpn#568

Change-Id: I47c992b6b73b1475cbff8a28f720cf50dc1fbe3e
---
M src/openvpn/forward.c
1 file changed, 14 insertions(+), 6 deletions(-)



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

Patch

diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
index 71b7167..0f89876 100644
--- a/src/openvpn/forward.c
+++ b/src/openvpn/forward.c
@@ -290,7 +290,6 @@ 
     struct buffer buf = alloc_buf_gc(len, &gc);
     if (tls_rec_payload(c->c2.tls_multi, &buf))
     {
-
         while (BLEN(&buf) > 1)
         {
             /* commands on the control channel are seperated by 0x00 bytes.
@@ -300,19 +299,28 @@ 
             if (cmdlen < BLEN(&buf))
             {
                 /* include the NUL byte and ensure NUL termination */
-                int cmdlen = (int)strlen(BSTR(&buf)) + 1;
+                int buflen = (int)strlen(BSTR(&buf)) + 1;
 
                 /* Construct a buffer that only holds the current command and
                  * its closing NUL byte */
-                struct buffer cmdbuf = alloc_buf_gc(cmdlen, &gc);
-                buf_write(&cmdbuf, BPTR(&buf), cmdlen);
+                struct buffer cmdbuf = alloc_buf_gc(buflen, &gc);
+                buf_write(&cmdbuf, BPTR(&buf), buflen);
+
+                /* Remove \r and \n at the end of the buffer to avoid
+                 * problems with scripts and other that add extra \r and \n */
+                while (BLEN(&cmdbuf)
+                       && (*(BEND(&cmdbuf) - 2) == '\r' || *(BEND(&cmdbuf) - 2)== '\n'))
+                {
+                    *(BEND(&cmdbuf) - 2) = '\0';
+                    buf_inc_len(&cmdbuf, -1);
+                }
 
                 /* check we have only printable characters or null byte in the
                  * command string and no newlines */
-                if (!string_check_buf(&buf, CC_PRINT | CC_NULL, CC_CRLF))
+                if (!string_check_buf(&cmdbuf, CC_PRINT | CC_NULL, CC_CRLF))
                 {
                     msg(D_PUSH_ERRORS, "WARNING: Received control with invalid characters: %s",
-                        format_hex(BPTR(&buf), BLEN(&buf), 256, &gc));
+                        format_hex(BPTR(&buf), BLEN(&cmdbuf), 256, &gc));
                 }
                 else
                 {