diff mbox series

[Openvpn-devel] options: check for blanks in fingerprints and reject string if found

Message ID 20210421234908.12817-1-a@unstable.cc
State Accepted
Headers
Series [Openvpn-devel] options: check for blanks in fingerprints and reject string if found |

Commit Message

Antonio Quartulli April 21, 2021, 1:49 p.m. UTC 
  From: Antonio Quartulli <antonio@openvpn.net>

A fingerprint is not expected to contains any blank (white space),
howeveri, the parser routine will still attempt parsing the octect
and ignore the space.

This means that a fingerprint like
5 :F0:A8:75:70:46:6E:0B:A2:31:53:88:0B:0E:8C:E4:8A:5E:BF:1E:08:16:16:41:63:2C:B5:F4:D2:73:9F:E5
will be parsed successfully.

Explcitly check for spaces in the various octects, before conversion,
and error out if any is found.

Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
---
 src/openvpn/options.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Comments

Arne Schwabe April 26, 2021, 12:59 a.m. UTC | #1 
Am 22.04.21 um 01:49 schrieb Antonio Quartulli:
> From: Antonio Quartulli <antonio@openvpn.net>
> 
> A fingerprint is not expected to contains any blank (white space),
> howeveri, the parser routine will still attempt parsing the octect
> and ignore the space.
> 
> This means that a fingerprint like
> 5 :F0:A8:75:70:46:6E:0B:A2:31:53:88:0B:0E:8C:E4:8A:5E:BF:1E:08:16:16:41:63:2C:B5:F4:D2:73:9F:E5
> will be parsed successfully.
> 
> Explcitly check for spaces in the various octects, before conversion,
> and error out if any is found.

Explicitly

> 
> Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
> ---
>  src/openvpn/options.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/src/openvpn/options.c b/src/openvpn/options.c
> index 817a1533..264fe383 100644
> --- a/src/openvpn/options.c
> +++ b/src/openvpn/options.c
> @@ -1099,6 +1099,18 @@ parse_hash_fingerprint(const char *str, int nbytes, int msglevel, struct gc_aren
>          bs[0] = *cp++;
>          bs[1] = *cp++;
>          bs[2] = 0;
> +
> +        /* the format string "%x" passed to sscanf will ignore any space and
> +         * will still try to parse the other character. However, this is not
> +         * expected format for a fingerprint, therefore explcitly check for

explicitly

> +         * blanks in the string and error out if any is found
> +         */
> +        if (bs[0] == ' ' || bs[1] == ' ')
> +        {
> +            msg(msglevel, "format error in hash fingerprint unexpected blank: %s",
> +                str);
> +        }
> +
>          byte = 0;
>          if (sscanf(bs, "%x", &byte) != 1)
>          {
> 

I would not have spend the time to fix this but since Antonio done it:

Acked-By: Arne Schwabe <arne@rfc2549.org>
Gert Doering April 26, 2021, 5:19 a.m. UTC | #2 
Your patch has been applied to the master branch.

That said, I find this function pretty insane, tbh.  Parsing a sequence
of double-digit hex bytes should be doable in half the lines of code,
and without needing to copy bytes to temp buffers to pass to scanf()...
(https://stackoverflow.com/questions/10156409/convert-hex-string-char-to-int)

commit 9e71cf13134c10bb7e952e3dadbcc79884f60b93
Author: Antonio Quartulli
Date:   Thu Apr 22 01:49:08 2021 +0200

     options: check for blanks in fingerprints and reject string if found

     Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
     Acked-by: Arne Schwabe <arne@rfc2549.org>
     Message-Id: <20210421234908.12817-1-a@unstable.cc>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg22182.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 817a1533..264fe383 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -1099,6 +1099,18 @@  parse_hash_fingerprint(const char *str, int nbytes, int msglevel, struct gc_aren
         bs[0] = *cp++;
         bs[1] = *cp++;
         bs[2] = 0;
+
+        /* the format string "%x" passed to sscanf will ignore any space and
+         * will still try to parse the other character. However, this is not
+         * expected format for a fingerprint, therefore explcitly check for
+         * blanks in the string and error out if any is found
+         */
+        if (bs[0] == ' ' || bs[1] == ' ')
+        {
+            msg(msglevel, "format error in hash fingerprint unexpected blank: %s",
+                str);
+        }
+
         byte = 0;
         if (sscanf(bs, "%x", &byte) != 1)
         {