[PATCH] shell.c: Dereference after null check (CID #26083)

[Gedare Bloom]

I need an explanation, I don't know if it is correct, or if someone
can open a shell without an 'out' FILE?

On Wed, Mar 10, 2021 at 12:10 PM Ryan Long <ryan.long at oarcorp.com> wrote:
>
> CID 26083: Dereference after null check in rtems_shell_login().
>
> Closes #4327
> ---
>  cpukit/libmisc/shell/shell.c | 196 ++++++++++++++++++++++---------------------
>  1 file changed, 99 insertions(+), 97 deletions(-)
>
> diff --git a/cpukit/libmisc/shell/shell.c b/cpukit/libmisc/shell/shell.c
> index 1e5962b..b724b1d 100644
> --- a/cpukit/libmisc/shell/shell.c
> +++ b/cpukit/libmisc/shell/shell.c
> @@ -683,109 +683,111 @@ static bool rtems_shell_login(rtems_shell_env_t *env, FILE * in,FILE * out)
>    int    c;
>    time_t t;
>
> -  if (out) {
> -    if ((env->devname[5]!='p')||
> -        (env->devname[6]!='t')||
> -        (env->devname[7]!='y')) {
> -      fd = fopen("/etc/issue","r");
> -      if (fd) {
> -        while ((c = fgetc(fd)) != EOF) {
> -          if (c=='@')  {
> -            switch (c = fgetc(fd)) {
> -              case 'L':
> -                fprintf(out,"%s", env->devname);
> -                break;
> -              case 'B':
> -                fprintf(out,"0");
> -                break;
> -              case 'T':
> -              case 'D':
> -                time(&t);
> -                fprintf(out,"%s",ctime(&t));
> -                break;
> -              case 'S':
> -                fprintf(out,"RTEMS");
> -                break;
> -              case 'V':
> -                fprintf(
> -                  out,
> -                  "%s\n%s",
> -                  rtems_get_version_string(),
> -                  rtems_get_copyright_notice()
> -                );
> -                break;
> -              case '@':
> -                fprintf(out,"@");
> -                break;
> -              default :
> -                fprintf(out,"@%c",c);
> -                break;
> -            }
> -          } else if (c=='\\')  {
> -            switch(c=fgetc(fd)) {
> -              case '\\': fprintf(out,"\\"); break;
> -              case 'b':  fprintf(out,"\b"); break;
> -              case 'f':  fprintf(out,"\f"); break;
> -              case 'n':  fprintf(out,"\n"); break;
> -              case 'r':  fprintf(out,"\r"); break;
> -              case 's':  fprintf(out," ");  break;
> -              case 't':  fprintf(out,"\t"); break;
> -              case '@':  fprintf(out,"@");  break;
> -            }
> -          } else {
> -            fputc(c,out);
> +  if (out == NULL) {
> +    return false;
> +  }
> +
> +  if ((env->devname[5]!='p')||
> +      (env->devname[6]!='t')||
> +      (env->devname[7]!='y')) {
> +    fd = fopen("/etc/issue","r");
> +    if (fd) {
> +      while ((c = fgetc(fd)) != EOF) {
> +        if (c=='@')  {
> +          switch (c = fgetc(fd)) {
> +            case 'L':
> +              fprintf(out,"%s", env->devname);
> +              break;
> +            case 'B':
> +              fprintf(out,"0");
> +              break;
> +            case 'T':
> +            case 'D':
> +              time(&t);
> +              fprintf(out,"%s",ctime(&t));
> +              break;
> +            case 'S':
> +              fprintf(out,"RTEMS");
> +              break;
> +            case 'V':
> +              fprintf(
> +                out,
> +                "%s\n%s",
> +                rtems_get_version_string(),
> +                rtems_get_copyright_notice()
> +              );
> +              break;
> +            case '@':
> +              fprintf(out,"@");
> +              break;
> +            default :
> +              fprintf(out,"@%c",c);
> +              break;
> +          }
> +        } else if (c=='\\')  {
> +          switch(c=fgetc(fd)) {
> +            case '\\': fprintf(out,"\\"); break;
> +            case 'b':  fprintf(out,"\b"); break;
> +            case 'f':  fprintf(out,"\f"); break;
> +            case 'n':  fprintf(out,"\n"); break;
> +            case 'r':  fprintf(out,"\r"); break;
> +            case 's':  fprintf(out," ");  break;
> +            case 't':  fprintf(out,"\t"); break;
> +            case '@':  fprintf(out,"@");  break;
>            }
> +        } else {
> +          fputc(c,out);
>          }
> -        fclose(fd);
>        }
> -    } else {
> -      fd = fopen("/etc/issue.net","r");
> -      if (fd) {
> -        while ((c=fgetc(fd))!=EOF) {
> -          if (c=='%')  {
> -            switch(c=fgetc(fd)) {
> -              case 't':
> -                fprintf(out,"%s", env->devname);
> -                break;
> -              case 'h':
> -                fprintf(out,"0");
> -                break;
> -              case 'D':
> -                fprintf(out," ");
> -                break;
> -              case 'd':
> -                time(&t);
> -                fprintf(out,"%s",ctime(&t));
> -                break;
> -              case 's':
> -                fprintf(out,"RTEMS");
> -                break;
> -              case 'm':
> -                fprintf(out,"(" CPU_NAME "/" CPU_MODEL_NAME ")");
> -                break;
> -              case 'r':
> -                fprintf(out,rtems_get_version_string());
> -                break;
> -              case 'v':
> -                fprintf(
> -                  out,
> -                  "%s\n%s",
> -                  rtems_get_version_string(),
> -                  rtems_get_copyright_notice()
> -                );
> -               break;
> -             case '%':fprintf(out,"%%");
> -               break;
> -             default:
> -                fprintf(out,"%%%c",c);
> -                break;
> -            }
> -          } else {
> -            fputc(c,out);
> +      fclose(fd);
> +    }
> +  } else {
> +    fd = fopen("/etc/issue.net","r");
> +    if (fd) {
> +      while ((c=fgetc(fd))!=EOF) {
> +        if (c=='%')  {
> +          switch(c=fgetc(fd)) {
> +            case 't':
> +              fprintf(out,"%s", env->devname);
> +              break;
> +            case 'h':
> +              fprintf(out,"0");
> +              break;
> +            case 'D':
> +              fprintf(out," ");
> +              break;
> +            case 'd':
> +              time(&t);
> +              fprintf(out,"%s",ctime(&t));
> +              break;
> +            case 's':
> +              fprintf(out,"RTEMS");
> +              break;
> +            case 'm':
> +              fprintf(out,"(" CPU_NAME "/" CPU_MODEL_NAME ")");
> +              break;
> +            case 'r':
> +              fprintf(out,rtems_get_version_string());
> +              break;
> +            case 'v':
> +              fprintf(
> +                out,
> +                "%s\n%s",
> +                rtems_get_version_string(),
> +                rtems_get_copyright_notice()
> +              );
> +              break;
> +            case '%':fprintf(out,"%%");
> +              break;
> +            default:
> +              fprintf(out,"%%%c",c);
> +              break;
>            }
> +        } else {
> +          fputc(c,out);
>          }
> -        fclose(fd);
>        }
> +      fclose(fd);
>      }
>    }
>
> --
> 1.8.3.1
>
> _______________________________________________
> devel mailing list
> devel at rtems.org
> http://lists.rtems.org/mailman/listinfo/devel