[app/xinit] Buffer overflow with many arguments.

Submitted by Walter Harms on Jan. 22, 2019, 4:55 p.m.

Details

Message ID 1401264109.55836.1548176105529@ox-groupware.bfs.de
State New
Series "Buffer overflow with many arguments."
Headers show

Commit Message

Walter Harms Jan. 22, 2019, 4:55 p.m.
> Tobias Stöckmann <tobias@stoeckmann.org> hat am 19. Januar 2019 um 20:37
> geschrieben:
> 
> 
> > hi,
> > nice catch.
> > 
> > instead of letting 98 magicly popup what is about
> > sizeof(serverargv)/sizeof(*serverargv) ?
> > Dito clientargv,
> > 
> > re,
> >  wh
> 
> There is still a pseudo-magical - 2 missing there, to keep space for the
> last NULL assignment.
> 
> But I'm fine with both. As long as 98 is the result. :)
> 
> 

this is my version, like your patch but the array limit is now calculated.
NTL the program needs some more.


Signed-off-by: Walter Harms <wharms@bfs.de>
---
 xinit.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Patch hide | download patch | download mbox

diff --git a/xinit.c b/xinit.c
index f826b7a..b93fe20 100644
--- a/xinit.c
+++ b/xinit.c
@@ -151,7 +151,6 @@  main(int argc, char *argv[])
     register char **ptr;
     pid_t pid;
     int client_given = 0, server_given = 0;
-    int client_args_given = 0, server_args_given = 0;
     int start_of_client_args, start_of_server_args;
     struct sigaction sa, si;
 #ifdef __APPLE__
@@ -174,7 +173,8 @@  main(int argc, char *argv[])
     }
     start_of_client_args = (cptr - client);
     while (argc && strcmp(*argv, "--")) {
-        client_args_given++;
+        if (cptr > clientargv + sizeof(clientargv)/sizeof(*clientargv)-2)
+           Fatalx("too many client arguments");
         *cptr++ = *argv++;
         argc--;
     }
@@ -202,7 +202,9 @@  main(int argc, char *argv[])
 
     start_of_server_args = (sptr - server);
     while (--argc >= 0) {
-        server_args_given++;
+        if (sptr > serverargv + sizeof(serverargv) /sizeof(*serverargv)-2 )
+            Fatalx("too many server arguments");
+
         *sptr++ = *argv++;
     }
     *sptr = NULL;