[PATCH] shell.c: Dereference after null check (CID #26083)
Gedare Bloom
gedare at rtems.org
Thu Mar 11 16:16:41 UTC 2021
On Thu, Mar 11, 2021 at 8:56 AM Ryan Long <ryan.long at oarcorp.com> wrote:
>
> 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.
>
works for me.
> -----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