ASUS DSL-AC3100 Router Firmware BCM Kernel Driver Bug

Time for some kernel bugs.

//********************************************************************************************
// misc. ioctl calls come to here. (flash, led, reset, kernel memory access, etc.)
//********************************************************************************************
static int board_ioctl( struct inode *inode, struct file *flip,
                       unsigned int command, unsigned long arg )
{
    int ret = 0;
    BOARD_IOCTL_PARMS ctrlParms;
    unsigned char ucaMacAddr[NVRAM_MAC_ADDRESS_LEN];
// ARCADYAN
    unsigned char ucaDectRfpi[NVRAM_ARCADYAN_DECT_RFPI_LEN];
    unsigned char ucaDectRxtun[NVRAM_ARCADYAN_DECT_RXTUN_LEN];
/// ARCADYAN

    switch (command) {
#if defined(BRCM_XDSL_DISTPOINT)
    case BOARD_IOCTL_FTTDP_DSP_BOOTER:  
        download_dsp_booter();
        break;
#endif
    case BOARD_IOCTL_FLASH_WRITE:
        if (copy_from_user((void*)&ctrlParms, (void*)arg, sizeof(ctrlParms)) == 0) {

            switch (ctrlParms.action) {
            case SCRATCH_PAD:
                if (ctrlParms.offset == -1)
                    ret =  kerSysScratchPadClearAll();
                else
                    ret = kerSysScratchPadSet(ctrlParms.string, ctrlParms.buf, ctrlParms.offset);
                break;
...
            case NVRAM:
            {
                NVRAM_DATA * pNvramData;

                /*
                 * Note: even though NVRAM access is protected by
                 * flashImageMutex at the kernel level, this protection will
                 * not work if two userspaces processes use ioctls to get
                 * NVRAM data, modify it, and then use this ioctl to write
                 * NVRAM data.  This seems like an unlikely scenario.
                 */
                mutex_lock(&flashImageMutex);
                if (NULL == (pNvramData = readNvramData()))
                {
                    mutex_unlock(&flashImageMutex);
                    return -ENOMEM;
                }
                if ( !strncmp(ctrlParms.string, "WLANFEATURE", 11 ) ) { //Wlan Data data
                    pNvramData->wlanParams[NVRAM_WLAN_PARAMS_LEN-1]= *(unsigned char *)(ctrlParms.string+11);
                    writeNvramDataCrcLocked(pNvramData);
                }
                else if ( !strncmp(ctrlParms.string, "WLANDATA", 8 ) ) { //Wlan Data data
                        int t_strlen=ctrlParms.strLen-8;
                        int nm=_get_wl_nandmanufacture();
                        if(nm<WLAN_MFG_PARTITION_HASSIZE) {
                                if(t_strlen>NVRAM_WLAN_PARAMS_LEN-1)
                                        t_strlen=NVRAM_WLAN_PARAMS_LEN-1;
                                memset((char *)pNvramData + ((size_t) &((NVRAM_DATA *)0)->wlanParams),
                                                0, sizeof(pNvramData->wlanParams)-1 );
                                memcpy( (char *)pNvramData + ((size_t) &((NVRAM_DATA *)0)->wlanParams),
                                                ctrlParms.string+8,
                                                t_strlen);

ok... where to begin?

lets keep the following in mind

typedef struct boardIoctParms
{
    char *string;
    char *buf;
    int strLen;
    int offset;
    BOARD_IOCTL_ACTION action;
    int result;
} BOARD_IOCTL_PARMS;

Lets go back over the code now:

    case BOARD_IOCTL_FLASH_WRITE:
        if (copy_from_user((void*)&ctrlParms, (void*)arg, sizeof(ctrlParms)) == 0) {

This is pretty straight forward. Copy the ioctl user args into kernelspace.

                else if ( !strncmp(ctrlParms.string, "WLANDATA", 8 ) ) { //Wlan Data data
                        int t_strlen=ctrlParms.strLen-8;

Huh? strncmp without copying the string from user to kernelspace? That doesn't seem good. Lets assume we can make this work.

                       int t_strlen=ctrlParms.strLen-8;
                        int nm=_get_wl_nandmanufacture();
                        if(nm<WLAN_MFG_PARTITION_HASSIZE) {
                                if(t_strlen>NVRAM_WLAN_PARAMS_LEN-1)
                                        t_strlen=NVRAM_WLAN_PARAMS_LEN-1;

t_strlen is a signed int. And ctrlParms.strLen is untrusted. Can we make t_strlen negative? We just need to pass the t_strlen > NVRAM_WLAN_PARAMS_LEN-1 check.

#define NVRAM_WLAN_PARAMS_LEN      256

Fortunately it's a signed integer, which means we can simply make ctrlParms.strLen less than 8. And we'll get a negative value into t_strlen.

                                 memset((char *)pNvramData + ((size_t) &((NVRAM_DATA *)0)->wlanParams),
                                                0, sizeof(pNvramData->wlanParams)-1 );
                                memcpy( (char *)pNvramData + ((size_t) &((NVRAM_DATA *)0)->wlanParams),
                                                ctrlParms.string+8,
                                                t_strlen);

And there we go. a memcpy with a negative size. This will cause memory corruption.


Popular posts from this blog

Empowering Women in Cybersecurity: InfoSect's 2024 Training Initiative

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

C++ Memory Corruption (std::vector) - part 2