Home
Consulting
Advisories
Software
Articles
Contact

Perl 5.22 VDir::MapPathA/W Out-of-bounds Reads and Buffer Over-reads

Vulnerabilities | Report

Perl 5.22 suffers from two out-of-bounds reads and multiple small buffer over-read vulnerabilities in the VDir::MapPathA and VDir::MapPathW functions that could potentially be exploited to achieve arbitrary code execution. The out-of-bounds read issues exist because the functions in question do not validate that the chr argument passed to DriveIndex, which calculates an index:

inline int DriveIndex(char chr)
{
    if (chr == '\\' || chr == '/')
        return ('Z'-'A')+1;
    return (chr | 0x20)-'a';
};
In the VDir::MapPathA function, DriveIndex is called with a potentially untrusted value, pInName, and the return value is then passed to GetDirA:
char *VDir::MapPathA(const char *pInName)
{   /*
     * possiblities -- relative path or absolute path with or without drive letter
     * OR UNC name
     */
    [...]

    if (pInName[1] == ':') {
	[...]
	}
	else {
	    /* relative path with drive letter */
	    strcpy(szBuffer, GetDirA(DriveIndex(*pInName)));
	    strcat(szBuffer, &pInName[2]);
	    if(strlen(szBuffer) > MAX_PATH)
		szBuffer[MAX_PATH] = '\0';

	    DoGetFullPathNameA(szBuffer, sizeof(szLocalBufferA), szLocalBufferA);
	}
    }
    else {
	[...]
    }

    return szLocalBufferA;
}
GetDirA then uses the unbounded index argument to index into dirTableA, a fixed length char pointer array.
inline const char *GetDirA(int index)
{
    char *ptr = dirTableA[index];
    if (!ptr) {
        /* simulate the existence of this drive */
        ptr = szLocalBufferA;
        ptr[0] = 'A' + index;
        ptr[1] = ':';
        ptr[2] = '\\';
        ptr[3] = 0;
    }
    return ptr;
};
In cases where index is attacker controlled, this behavior can be used to read outside of the dirTableA array. This is especially problematic because the value returned is then copied to a fixed length buffer using strcpy:
strcpy(szBuffer, GetDirA(DriveIndex(*pInName)));
If an attacker can manipulate the layout of memory to trick GetDirA into returning a string larger than szBuffer, a buffer overflow will occur. The issue in VDir::MapPathW is nearly identical.. Further, multiple small and less critical buffer over-reads exist in both VDir::MapPathA and VDir::MapPathW:
char *VDir::MapPathA(const char *pInName)
{   /*
     * possiblities -- relative path or absolute path with or without drive letter
     * OR UNC name
     */
    [...]

    if (!length) <<< Check here confirms the buffer is at least of length 2 (including null) before continuing execution.
	return (char*)pInName;

    [...]

    if (pInName[1] == ':') { <<< While technically no over-read can occur here, pInName is a single character, this checks the null terminator.
	/* has drive letter */
	if (IsPathSep(pInName[2])) { <<<< This could cause an over-read because the string could possibly be of length 2 (including null).
	    /* absolute with drive letter */
	    DoGetFullPathNameA((char*)pInName, sizeof(szLocalBufferA), szLocalBufferA);
	}
	else {
	    /* relative path with drive letter */
	    strcpy(szBuffer, GetDirA(DriveIndex(*pInName)));
	    strcat(szBuffer, &pInName[2]); <<<< This could cause an over-read for the same reason.
	    if(strlen(szBuffer) > MAX_PATH)
		szBuffer[MAX_PATH] = '\0';

	    DoGetFullPathNameA(szBuffer, sizeof(szLocalBufferA), szLocalBufferA);
	}
    }
    else {
	[...]
	}
    }

    return szLocalBufferA;
}
To observe the out-of-bounds read vulnerability in VDir::MapPathA, the following script can be executed while Perl is under a debugger:
print glob "]:";
Which will result in an exception similar to the following:

(f78.1dd8): Access violation - code c0000005 (first chance)
First chance exceptions are reported before any exception handling.
This exception may be expected and handled.
eax=0081d62c ebx=0081dae2 ecx=765c7377 edx=7eff3920 esi=0081dae0 edi=0081d62c
eip=747613a0 esp=0081d608 ebp=74744fac iopl=0 nv up ei pl nz na pe nc
cs=0023 ss=002b ds=002b es=002b fs=0053 gs=002b efl=00010206
MSVCR110!strcat+0x71:
747613a0 8a11 mov dl,byte ptr [ecx] ds:002b:765c7377=??
0:000> k
ChildEBP RetAddr
0081d608 709059ea MSVCR110!strcat+0x71
0081d940 7090688e perl523!VDir::MapPathA+0xdd
0081d94c 7090e295 perl523!PerlDirMapPathA+0x1f
0081dab4 70906736 perl523!win32_stat+0x6e
0081dac0 72541f60 perl523!PerlLIOLstat+0xd
0081dee4 7254181b Glob!g_lstat+0x72
0081df6c 725415e7 Glob!glob2+0x7a
0081efb0 72541141 Glob!glob0+0x181
0081f7cc 725420e8 Glob!bsd_glob+0x11d
0081f814 72542929 Glob!doglob+0x3f
0081f850 72542391 Glob!csh_glob+0x4a2
0081f894 708a05f4 Glob!iterate+0x1e8
0081f8ac 708cd3d8 perl523!Perl_pp_glob+0x19a
0081f8b8 70871fd8 perl523!Perl_runops_standard+0xc
0081f8cc 70871ef8 perl523!S_run_body+0xdf
0081f938 70908290 perl523!perl_run+0x1e6
0081fb68 00fe1216 perl523!RunPerl+0xbc
0081fba8 76de3744 perl!__tmainCRTStartup+0xfd
0081fbbc 77b7a064 KERNEL32!BaseThreadInitThunk+0x24
0081fc04 77b7a02f ntdll!__RtlUserThreadStart+0x2f
0081fc14 00000000 ntdll!_RtlUserThreadStart+0x1b

To fix the issue, it is recommended that the VDir::MapPathA and VDir::MapPathW functions validate the drive letter to ensure no out-of-bounds reads occur, and also check the length of the pInName argument to ensure no buffer over-reads occur. A proposed patch is attached. However, the patch only addresses the issues in VDir::MapPathA because it was not immediately clear how to hit VDir::MapPathW for the purpose of testing.

Credit: John Leitch (john@autosectools.com), Bryce Darling



Copyright © 2018 AutoSec Tools LLC