[PATCH] shell: Out-of-bounds access

[Gedare Bloom]

On Thu, Sep 5, 2013 at 12:18 PM, Gedare Bloom <gedare at rtems.org> wrote:
> In case the length of cwd path plus the userScriptName exceeds
> PATH_MAX (255), the strncat calls will overflow scriptFile. Also
> check for getcwd failure.
> ---
>  cpukit/libmisc/shell/shell_script.c |   23 +++++++++++++++--------
>  1 files changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/cpukit/libmisc/shell/shell_script.c b/cpukit/libmisc/shell/shell_script.c
> index 98d0a5b..c055e3f 100644
> --- a/cpukit/libmisc/shell/shell_script.c
> +++ b/cpukit/libmisc/shell/shell_script.c
> @@ -50,6 +50,7 @@ static int findOnPATH(
>  )
>  {
>    int sc;
> +  char *cwd;
>
>    /*
>     *  If the user script name starts with a / assume it is a fully
> @@ -65,14 +66,20 @@ static int findOnPATH(
>       */
>
>      /* XXX should use strncat but what is the limit? */
> -    getcwd( scriptFile, PATH_MAX );
> -    strncat( scriptFile, "/", PATH_MAX );
> -    strncat(
> -      scriptFile,
> -      ( (userScriptName[0] == '.' && userScriptName[1] == '/') ?
> -         &userScriptName[2] : userScriptName),
> -      PATH_MAX
> -    );
> +    cwd = getcwd( scriptFile, PATH_MAX );
> +    if ( cwd != NULL ) {
> +      int cwdlen = strnlen( scriptFile, PATH_MAX );
> +
> +      strncat( scriptFile, "/", PATH_MAX - cwdlen );
> +      strncat(
> +          scriptFile,
> +          ( (userScriptName[0] == '.' && userScriptName[1] == '/') ?
> +            &userScriptName[2] : userScriptName),
> +          PATH_MAX - cwdlen - 1
> +          );
A different fix for this would be to use strlcat() instead, which will
compute the space available in the scriptFile destination.

> +    } else {
> +      return -1;
> +    }
>    }
>
>    sc = access( scriptFile, R_OK );
> --
> 1.7.1
>