#392 Buffer overflow in DOS_Shell::Which()

0.74
fixed
None
1
2015-01-09
2013-04-27
No

Here:

    i_path = 0; /* reset writer */
    while(*pathenv && (*pathenv !=';') && (i_path < DOS_PATHLENGTH) )
        path[i_path++] = *pathenv++;

    if(i_path == DOS_PATHLENGTH) {
        /* If max size. move till next ; and terminate path */
        while(*pathenv != ';') 
            pathenv++;
        path[DOS_PATHLENGTH - 1] = 0;
    } else path[i_path] = 0;

If we collect DOS_PATHLENGTH characters into path[], we'll execute the code under "if". If we've reached the end of the string in pathenv, we'll happily advance the pointer beyond the end of the buffer until it points to ';' or until it causes a crash while looking for a ';'. A hang is also possible.

I suggest moving the useless first check from here:

    /* remove ; and ;; at the beginning. (and from the second entry etc) */
    while(*pathenv && (*pathenv ==';'))
        pathenv++;

to where it really needs to be, to here:

        /* If max size. move till next ; and terminate path */
        while(*pathenv != ';') 
            pathenv++;

like so:

    /* remove ; and ;; at the beginning. (and from the second entry etc) */
    while(*pathenv ==';')
        pathenv++;

...

        /* If max size. move till next ; and terminate path */
        while(*pathenv && *pathenv != ';') 
            pathenv++;

Discussion

  • Peter Veenstra

    Peter Veenstra - 2015-01-09

    You are right.
    Thanks.

     
  • Peter Veenstra

    Peter Veenstra - 2015-01-09
    • status: open --> fixed
    • assigned_to: Peter Veenstra
     

Log in to post a comment.