[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Security in a shell that starts ssh



On Wed, Jun 13, 2001 at 10:57:08AM -0500, Steve Greenland wrote:
> Tim, good fixups, a few C coding/style nitpicks:
> 
> On 12-Jun-01, 17:57 (CDT), Tim van Erven <tripudium@chello.nl> wrote: 
> > #include <stdio.h>
> 
> #include <unistd.h> /* For execlp */
> #include <stdlib.h> /* For exit */
> 
> > int main()
> 
> int main(void)   /* () != (void) in C */
> 
> > {
> >   char	name[21]; /* Should be macro (#define NAMELEN 21) */
> > 
> >   printf("Login as: ");
> >   fflush(stdout);
> > 
> >   if(fgets(name, 21, stdin)) {
> >     /* if(name[strlen(name) - 1] != '\n') */
> 
>      if(name[strlen(name) - 1] != '\n') {
>

Possible access to unallocated memory if "\0\n" supplied as input.

> >       fprintf(stderr, "Username to long.\n");
> >     /* else { */
> 
>      } else {
> 
> >       name[strlen(name) - 1] = '\0';
> >       execlp("/usr/bin/ssh", "ssh", "-l", name, "foo.foo.es", (char *)0);
> >     }
> >   }
> > 
> >   /* return 0; */
> 
>     exit(EXIT_SUCCESS); /* return doesn't call atexit() registered functions,
>                            which doesn't apply in this case, but it's a good
>                            habit to get into */
>

Wrong comment. Returning from main _does_ call atexit() registered
functions.

> > }
> 
> 
> You also should should make sure name doesn't contain any spaces: as
> written I can pass additional options to ssh. Also, for this kind of
> application you really ought to be checking the error conditions for
> *every* library call.
> 

Spaces and other shell metacharecters are irrelevant in this case, since
executed command won't undergo shell interpretation.

-- 
dg



Reply to: