On Fri, Jun 28, 2019 at 03:19:01PM +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 bridge name to IFNAMSIZ, including null byte.
Can you elaborate on the way to exploit this as I'm not seeing any way that doesn't involve mis-configuration of the ACL config file data. Currently the code has: const char *bridge = NULL; ... bridge = &argv[index][5]; ... if (strcmp(bridge, acl_rule->iface) == 0) { So the user suplied "--br=bridge" name will never be truncated. The ACLRule struct has iface[IFNAMSIZ] so it is possible that we truncate the bridge name that's in the bridge.conf file if the admin put in a name that was too long. This is simply an admin mis-configuration, since it is impossible for a bridge to actually exist that has a name longer than IFNAMSIZ. I think we simply need to report a hard error when reading the bridge.conf file if the name we find does not fit into IFNAMSIZ. > > Reported-by: Riccardo Schirone <rschi...@redhat.com> > Signed-off-by: Prasad J Pandit <p...@fedoraproject.org> > --- > qemu-bridge-helper.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/qemu-bridge-helper.c b/qemu-bridge-helper.c > index f9940deefd..2eca8c5cc4 100644 > --- a/qemu-bridge-helper.c > +++ b/qemu-bridge-helper.c > @@ -246,7 +246,7 @@ int main(int argc, char **argv) > if (strcmp(argv[index], "--use-vnet") == 0) { > use_vnet = 1; > } else if (strncmp(argv[index], "--br=", 5) == 0) { > - bridge = &argv[index][5]; > + bridge = strndup(&argv[index][5], IFNAMSIZ - 1); This is not fixing the real problem imho > } else if (strncmp(argv[index], "--fd=", 5) == 0) { > unixfd = atoi(&argv[index][5]); > } else { > -- > 2.21.0 > 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 :|