Re: [Qemu-devel] [PATCH v3 1/3] qemu-bridge-helper: restrict interface name to IFNAMSIZ

[Li Qiang]

P J P <address@hidden> 于2019年7月1日周一 下午8:38写道：

> From: Prasad J Pandit <address@hidden>
>
> 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 <address@hidden>
> Signed-off-by: Prasad J Pandit <address@hidden>
> ---
>  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 <address@hidden>




>          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
>
>