Re: [Qemu-devel] [PATCH] fmops: fix off-by-one in AR_TABLE and DR_TABLE array size

[Philippe Mathieu-Daudé]

Hi Gerd,

On 30/10/18 9:23, Gerd Hoffmann wrote:

Can you add your previous patch description,

  We have a lovely, guest-triggerable buffer overflow in opl2 emulation.

  Reproducer:
      outw(0xff60, 0x220);
      outw(0x1020, 0x220);
      outw(0xffb0, 0x220);
  Result:
      Will overflow FM_OPL->AR_TABLE[] (see hw/audio/fmopl.[ch])

And Prasad Pandit triggering flow:

  In set_ar_dr

    SLOT->AR = ar ? &OPL->AR_TABLE[ar<<2] : RATE_0;

  SLOT->AR is set to point to OPL->DR_TABLE[60] and while so if s->ksr
  is set to 15, in CALC_FCSLOT()

  SLOT->evsa = SLOT->AR[ksr];  <= accesses OPL->AR_TABLE[60 + 15];

Thanks.

> Fixes: CVE-2018-???
> Cc: P J P <address@hidden>
> Reported-by: Wangjunqing <address@hidden>
> Suggested-by: Paolo Bonzini <address@hidden>
> Signed-off-by: Gerd Hoffmann <address@hidden>

Reviewed-by: Philippe Mathieu-Daudé <address@hidden>

> ---
>   hw/audio/fmopl.h | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/audio/fmopl.h b/hw/audio/fmopl.h
> index e7e578a48e..7199afaa3c 100644
> --- a/hw/audio/fmopl.h
> +++ b/hw/audio/fmopl.h
> @@ -72,8 +72,8 @@ typedef struct fm_opl_f {
>         /* Rhythm sention */
>         uint8_t rhythm;         /* Rhythm mode , key flag */
>         /* time tables */
> -       int32_t AR_TABLE[75];   /* atttack rate tables */
> -       int32_t DR_TABLE[75];   /* decay rate tables   */
> +       int32_t AR_TABLE[76];   /* atttack rate tables */
> +       int32_t DR_TABLE[76];   /* decay rate tables   */
>         uint32_t FN_TABLE[1024];  /* fnumber -> increment counter */
>         /* LFO */
>         int32_t *ams_table;