* Michael S. Tsirkin (m...@redhat.com) wrote:
> CVE-2013-4532
>
> s->tx_fifo_len is read from the wire and later used as an index into
> s->tx_fifo[] when a DATA command is issued by the guest. If
> s->tx_fifo_len is greater than the length of s->tx_fifo[], or less
> than 0, the buffer can be overrun/underrun by arbitrary data written out
> by the guest upon resuming its execution.
>
> Fix this by failing migration if the value from the wire would make
> guest access the array out of bounds.
>
> Reported-by: Michael Roth <mdr...@linux.vnet.ibm.com>
> Reported-by: Peter Maydell <peter.mayd...@linaro.org>
> Signed-off-by: Michael S. Tsirkin <m...@redhat.com>
> ---
> hw/net/stellaris_enet.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/hw/net/stellaris_enet.c b/hw/net/stellaris_enet.c
> index 182fd3e..aed00fd 100644
> --- a/hw/net/stellaris_enet.c
> +++ b/hw/net/stellaris_enet.c
> @@ -358,7 +358,7 @@ static void stellaris_enet_save(QEMUFile *f, void *opaque)
> static int stellaris_enet_load(QEMUFile *f, void *opaque, int version_id)
> {
> stellaris_enet_state *s = (stellaris_enet_state *)opaque;
> - int i, v;
> + int i, v, sz;
>
> if (version_id != 1)
> return -EINVAL;
> @@ -374,7 +374,13 @@ static int stellaris_enet_load(QEMUFile *f, void
> *opaque, int version_id)
> s->mrxd = qemu_get_be32(f);
> s->np = qemu_get_be32(f);
> s->tx_frame_len = qemu_get_be32(f);
> - s->tx_fifo_len = qemu_get_be32(f);
> + v = qemu_get_be32(f);
> + /* How many bytes does data use in tx fifo. */
> + sz = s->tx_frame_len == -1 ? 2 : 4;
> + if (v < 0 || v >= ARRAY_SIZE(s->tx_fifo) - sz) {
> + return -EINVAL;
> + }
> + s->tx_fifo_len = v;
> qemu_get_buffer(f, s->tx_fifo, sizeof(s->tx_fifo));
I don't understand the magic '2' in that ?2:4 - but there again the 'DATA' write
case is pretty hairy.
Dave
> for (i = 0; i < 31; i++) {
> s->rx[i].len = qemu_get_be32(f);
> --
> MST
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
