Bug 475733 (CVE-2008-5702)

From Zvonimir Rakamaric:
Steps to a possible buffer_underflow bug in ib700wdt.c, function ibwdt_ioctl:

1. new margin is loaded using get_user(new_margin, p) in case WDIOC_SETTIMEOUT in the function ibwdt_ioctl. Important: assume that new_margin == 30
2. ibwdt_set_heartbeat(new_margin) is called
3. the check "if ((t < 0) || (t > 30))" in ibwdt_set_heartbeat is not going to fail because t == 30 (t is new_margin)
4. in the loop, the check wd_times[i] > t is never going to be true because none of the wd_times are greater than the value of t (i.e. 30)
5. we are exiting the loop with i == -1 and therefore setting wd_margin to -1 in the line wd_margin = i;
6. we are returning from ibwdt_set_heartbeat and wd_margin is -1
7. we fall through to case WDIOC_GETTIMEOUT
8. we access the wd_times array with wd_margin == -1 on line "return put_user(wd_times[wd_margin], p);"

[...]
Furthermore, here are the pieces of code which are causing this
buffer-underflow bug in ib700wdt.c:

static int wd_times[] = {
        30,     /* 0x0 */
        28,     /* 0x1 */
        26,     /* 0x2 */
        24,     /* 0x3 */
        22,     /* 0x4 */
        20,     /* 0x5 */
        18,     /* 0x6 */
        16,     /* 0x7 */
        14,     /* 0x8 */
        12,     /* 0x9 */
        10,     /* 0xA */
        8,      /* 0xB */
        6,      /* 0xC */
        4,      /* 0xD */
        2,      /* 0xE */
        0,      /* 0xF */
};

function ibwdt_ioctl:
.....
  case WDIOC_SETTIMEOUT:
    if (get_user(new_margin, p))
      return -EFAULT;
    if (ibwdt_set_heartbeat(new_margin))
      return -EINVAL;
    ibwdt_ping();
    /* Fall */

  case WDIOC_GETTIMEOUT:
    return put_user(wd_times[wd_margin], p);
.....

function ibwdt_set_heartbeat:
static int
ibwdt_set_heartbeat(int t)
{
        int i;

        if ((t < 0) || (t > 30))
                return -EINVAL;

        for (i = 0x0F; i > -1; i--)
                if (wd_times[i] > t)
                        break;
        wd_margin = i;
        return 0;
}