[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 1/3] qemu-bridge-helper: restrict interface n
From: |
Daniel P . Berrangé |
Subject: |
Re: [Qemu-devel] [PATCH v2 1/3] qemu-bridge-helper: restrict interface name to IFNAMSIZ |
Date: |
Mon, 1 Jul 2019 10:43:53 +0100 |
User-agent: |
Mutt/1.12.0 (2019-05-25) |
On Mon, Jul 01, 2019 at 02:39:02PM +0530, P J P wrote:
> From: Prasad J Pandit <address@hidden>
>
> The interface names in qemu-bridge-helper are 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 ACLs 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 v2: report an error and exit
> -> https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg06239.html
>
> diff --git a/qemu-bridge-helper.c b/qemu-bridge-helper.c
> index f9940deefd..8ec0a65174 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 (strcmp(cmd, "include") && strlen(arg) >= IFNAMSIZ) {
> + fprintf(stderr, "name `%s' too long: %lu\n", arg, strlen(arg));
strlen returns size_t, which does not match %lu - it needs %zu - we can
ignore the non-portability of %zu to windows, since this code is UNIX
only.
I'd prefer also !g_str_equal(cmd, "include") as to me it reads more
easily.
> + 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: %lu\n", bridge, strlen(bridge));
> + return EXIT_FAILURE;
> + }
%zu too
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
- [Qemu-devel] [PATCH v2 0/3] restrict bridge interface name to IFNAMSIZ, P J P, 2019/07/01
- [Qemu-devel] [PATCH v2 1/3] qemu-bridge-helper: restrict interface name to IFNAMSIZ, P J P, 2019/07/01
- Re: [Qemu-devel] [PATCH v2 1/3] qemu-bridge-helper: restrict interface name to IFNAMSIZ,
Daniel P . Berrangé <=
- [Qemu-devel] [PATCH v2 3/3] net: tap: restrict bridge name to IFNAMSIZ, P J P, 2019/07/01
- [Qemu-devel] [PATCH v2 2/3] qemu-bridge-helper: move repeating code in parse_acl_file, P J P, 2019/07/01
- Re: [Qemu-devel] [PATCH v2 0/3] restrict bridge interface name to IFNAMSIZ, no-reply, 2019/07/01
- Re: [Qemu-devel] [PATCH v2 0/3] restrict bridge interface name to IFNAMSIZ, no-reply, 2019/07/01