On Mon, Jul 01, 2019 at 02:39:02PM +0530, P J P wrote:
> From: Prasad J Pandit <p...@fedoraproject.org>
>
> 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 <rschi...@redhat.com>
> Signed-off-by: Prasad J Pandit <p...@fedoraproject.org>
> ---
> 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 :|
