ASUS DSL-AC3100 Router Firmware PPP bug

Lets look at the following code in the current version of ppp on Linux.

static void
cbcp_recvreq(us, pckt, pcktlen)
    cbcp_state *us;
    u_char *pckt;
    int pcktlen;
{
    u_char type, opt_len, delay, addr_type;
    char address[256];
    int len = pcktlen;

    address[0] = 0;

    while (len >= 2) {
        dbglog("length: %d", len);

        GETCHAR(type, pckt);
        GETCHAR(opt_len, pckt);
        if (opt_len < 2 || opt_len > len)
            break;

        if (opt_len > 2)
            GETCHAR(delay, pckt);

        us->us_allowed |= (1 << type);

        switch(type) {
        case CB_CONF_NO:
            dbglog("no callback allowed");
            break;

        case CB_CONF_USER:
            dbglog("user callback allowed");
            if (opt_len > 4) {
                GETCHAR(addr_type, pckt);
                memcpy(address, pckt, opt_len - 4);
                address[opt_len - 4] = 0;
                if (address[0])
                    dbglog("address: %s", address);
            }
            break;

        case CB_CONF_ADMIN:
            dbglog("user admin defined allowed");
            break;

        case CB_CONF_LIST:
            break;
        }
        len -= opt_len;
    }
    if (len != 0) {
        if (debug)
            dbglog("cbcp_recvreq: malformed packet (%d bytes left)", len);
        return;
    }

    cbcp_resp(us);
}

Now I draw your attention to the loop condition. while (len >= 2). Lets look at the router firmware code:

    while (len) {
        dbglog("length: %d", len);

        GETCHAR(type, pckt);
        GETCHAR(opt_len, pckt);

        if (opt_len > 2)
            GETCHAR(delay, pckt);

        us->us_allowed |= (1 << type);

        switch(type) {
        case CB_CONF_NO:
            dbglog("no callback allowed");
            break;

        case CB_CONF_USER:
            dbglog("user callback allowed");
            if (opt_len > 4) {
                GETCHAR(addr_type, pckt);
                memcpy(address, pckt, opt_len - 4);
                address[opt_len - 4] = 0;
                if (address[0])
                    dbglog("address: %s", address);
            }
            break;

        case CB_CONF_ADMIN:
            dbglog("user admin defined allowed");
            break;

        case CB_CONF_LIST:
            break;
        }
        len -= opt_len;
    }

What's going to happen if the opt_len's don't add up to the packet size? Lets think of when len is 1 and opt_len is 2. It'll wrap around to a non zero negative. And the loop will keep iterating.

Clearly buggy.

Popular posts from this blog

Empowering Women in Cybersecurity: InfoSect's 2024 Training Initiative

C++ Memory Corruption (std::string) - part 4

Pointer Compression in V8