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: