QcowFormatInspector feature check checks wrong bitmask

[Olaf Seibert]

Consider the code at https://opendev.org/openstack/nova/src/branch/master/nova/image/format_inspector.py#L330

        # This is the maximum byte number we should expect any bits to be set
        max_byte = self.I_FEATURES_MAX_BIT // 8

        # The flag bytes are in big-endian ordering, so if we process
        # them in index-order, they're reversed
        for i, byte_num in enumerate(reversed(range(self.I_FEATURES_LEN))):
            if byte_num == max_byte:
                # If we're in the max-allowed byte, allow any bits less than
                # the maximum-known feature flag bit to be set
                allow_mask = ((1 << self.I_FEATURES_MAX_BIT) - 1)
...

            if i_features[i] & ~allow_mask:
                LOG.warning('Found unknown feature bit in byte %i: %s/%s',
                            byte_num, bin(i_features[byte_num] & ~allow_mask),
                            bin(allow_mask))

If I_FEATURES_MAX_BIT is 8 or larger, the allow_mask created for the max_byte is incorrect.
There should be a MOD 8 in the calculation.
As it is, the created allow_mask will have all bits set in the lower 8 bits and allow all bits in the max_byte byte.

By sheer luck, the current value of I_FEATURES_MAX_BIT is only 4, which keeps this from being an active security vulnerability (only a potential one in the future, if this is not fixed before I_FEATURES_MAX_BIT is increased).

[Olaf Seibert (oseibert-sys11)]

Same at https://opendev.org/openstack/cinder/src/branch/master/cinder/image/format_inspector.py#L328

and

https://opendev.org/openstack/glance/src/branch/master/glance/common/format_inspector.py#L327

[Olaf Seibert (oseibert-sys11)]

Also, let's quickly mention that the inspector assumes the v3 file format. Qcow v2 files do not even have the feature bits, yet the inspector unconditionally looks for them.

See https://gitlab.com/qemu-project/qemu/-/blob/master/docs/interop/qcow2.txt#L88

[sean mooney (sean-k-mooney)]

we currently have no intention to extend the in tree copies of the format inspector to increase
I_FEATURES_MAX_BIT beyond its hardcoded value fo 4

this is a valid design consideration for the oslo.utils version

and yes we are assuming qcow v3 implicitly most distos moved to v3 quite some time ago.
we do not officially declare which versions of qcow images we support but we have stopped testing with v2
when we moved to ubuntu 22.04 and centos 9 streams since they implicitly convert the images to v3.

[Olaf Seibert (oseibert-sys11)]

One of our customers did try to use a Qcow v2 file in the last weeks, so those images are certainly around "in the wild". They probably got it from some old source, but that should not matter.

[Dan Smith (danms)]

Yeah, agree with Sean: we're being as strict as possible for a reason (i.e. advice from the QEMU team). If you want to propose changes (against the oslo version) to support earlier versions of the spec without compromising the protections of the later/current versions we can certainly review those.