[PATCH] shell.c: Dereference after null check (CID #26083)
Ryan Long
ryan.long at oarcorp.com
Thu Mar 11 15:56:48 UTC 2021
After talking with Joel, we determined that 'out' shouldn’t be NULL by the time it gets to rtems_shell_login().
We think that we should put in an assert to check whether stdin and stdout is NULL at https://git.rtems.org/rtems/tree/cpukit/libmisc/shell/shell.c#n968.
Instead of the check that I put in for this patch, we think it's better to just put another assert at https://git.rtems.org/rtems/tree/cpukit/libmisc/shell/shell.c#n791 for 'out' out of paranoia and to get rid of the Coverity issue.
-----Original Message-----
From: Gedare Bloom <gedare at rtems.org>
Sent: Wednesday, March 10, 2021 4:50 PM
To: Ryan Long <ryan.long at oarcorp.com>
Cc: devel at rtems.org
Subject: Re: [PATCH] shell.c: Dereference after null check (CID #26083)
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
More information about the devel
mailing list