diff mbox series

[v3,1/3] qemu-bridge-helper: restrict interface name to IFNAMSIZ

Message ID 20190701123558.30512-2-ppandit@redhat.com (mailing list archive)
State New, archived
Headers show
Series restrict bridge interface name to IFNAMSIZ | expand

Commit Message

Prasad Pandit July 1, 2019, 12:35 p.m. UTC
From: Prasad J Pandit <pjp@fedoraproject.org>

The network interface name in Linux is defined to be of size
IFNAMSIZ(=16), including the terminating null('\0') byte.
The same is applied to interface names read from 'bridge.conf'
file to form ACL rules. If user supplied '--br=bridge' name
is not restricted to the same length, it could lead to ACL bypass
issue. Restrict interface name to IFNAMSIZ, including null byte.

Reported-by: Riccardo Schirone <rschiron@redhat.com>
Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
---
 qemu-bridge-helper.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

Update v3: use g_str_equal() and %zu for strlen()
  -> https://lists.gnu.org/archive/html/qemu-devel/2019-07/msg00072.html

Comments

Li Qiang July 1, 2019, 3:22 p.m. UTC | #1
P J P <ppandit@redhat.com> 于2019年7月1日周一 下午8:38写道:

> From: Prasad J Pandit <pjp@fedoraproject.org>
>
> The network interface name in Linux is defined to be of size
> IFNAMSIZ(=16), including the terminating null('\0') byte.
> The same is applied to interface names read from 'bridge.conf'
> file to form ACL rules. If user supplied '--br=bridge' name
> is not restricted to the same length, it could lead to ACL bypass
> issue. Restrict interface name to IFNAMSIZ, including null byte.
>
> Reported-by: Riccardo Schirone <rschiron@redhat.com>
> Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
> ---
>  qemu-bridge-helper.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> Update v3: use g_str_equal() and %zu for strlen()
>   -> https://lists.gnu.org/archive/html/qemu-devel/2019-07/msg00072.html
>
> diff --git a/qemu-bridge-helper.c b/qemu-bridge-helper.c
> index f9940deefd..e90c22f07d 100644
> --- a/qemu-bridge-helper.c
> +++ b/qemu-bridge-helper.c
> @@ -109,6 +109,13 @@ static int parse_acl_file(const char *filename,
> ACLList *acl_list)
>          }
>          *argend = 0;
>
> +        if (!g_str_equal(cmd, "include") && strlen(arg) >= IFNAMSIZ) {
> +            fprintf(stderr, "name `%s' too long: %zu\n", arg,
> strlen(arg));
> +            fclose(f);
> +            errno = EINVAL;
> +            return -1;
> +        }
> +
>

g_str_equal is not consistent style with the other 'strcmp' in this file.
With g_str_equal or strcmp:

Reviewed-by: Li Qiang <liq3ea@gmail.com>




>          if (strcmp(cmd, "deny") == 0) {
>              acl_rule = g_malloc(sizeof(*acl_rule));
>              if (strcmp(arg, "all") == 0) {
> @@ -259,6 +266,10 @@ int main(int argc, char **argv)
>          usage();
>          return EXIT_FAILURE;
>      }
> +    if (strlen(bridge) >= IFNAMSIZ) {
> +        fprintf(stderr, "name `%s' too long: %zu\n", bridge,
> strlen(bridge));
> +        return EXIT_FAILURE;
> +    }
>
>      /* parse default acl file */
>      QSIMPLEQ_INIT(&acl_list);
> --
> 2.21.0
>
>
diff mbox series

Patch

diff --git a/qemu-bridge-helper.c b/qemu-bridge-helper.c
index f9940deefd..e90c22f07d 100644
--- a/qemu-bridge-helper.c
+++ b/qemu-bridge-helper.c
@@ -109,6 +109,13 @@  static int parse_acl_file(const char *filename, ACLList *acl_list)
         }
         *argend = 0;
 
+        if (!g_str_equal(cmd, "include") && strlen(arg) >= IFNAMSIZ) {
+            fprintf(stderr, "name `%s' too long: %zu\n", arg, strlen(arg));
+            fclose(f);
+            errno = EINVAL;
+            return -1;
+        }
+
         if (strcmp(cmd, "deny") == 0) {
             acl_rule = g_malloc(sizeof(*acl_rule));
             if (strcmp(arg, "all") == 0) {
@@ -259,6 +266,10 @@  int main(int argc, char **argv)
         usage();
         return EXIT_FAILURE;
     }
+    if (strlen(bridge) >= IFNAMSIZ) {
+        fprintf(stderr, "name `%s' too long: %zu\n", bridge, strlen(bridge));
+        return EXIT_FAILURE;
+    }
 
     /* parse default acl file */
     QSIMPLEQ_INIT(&acl_list);