Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SECURITY: Buffer Overflow in ConnectClientToUnixSock() #291

Closed
javadiahmad opened this issue Mar 30, 2019 · 2 comments
Closed

SECURITY: Buffer Overflow in ConnectClientToUnixSock() #291

javadiahmad opened this issue Mar 30, 2019 · 2 comments

Comments

@javadiahmad
Copy link

javadiahmad commented Mar 30, 2019

Hi @bk138

less /usr/include/linux/un.h
/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
#ifndef _LINUX_UN_H
#define _LINUX_UN_H

#include <linux/socket.h>

#define UNIX_PATH_MAX   108

struct sockaddr_un {
        __kernel_sa_family_t sun_family; /* AF_UNIX */
        char sun_path[UNIX_PATH_MAX];   /* pathname */
};

#define SIOCUNIXFILE (SIOCPROTOPRIVATE + 0) /* open a socket file with O_PATH */

#endif /* _LINUX_UN_H */

We see char sun_path[UNIX_PATH_MAX]; /* pathname */ of sockaddr_un that UNIX_PATH_MAX=108

Issue :

We see of code sockets.c of function ConnectClientToUnixSock() L464, there is buffer overflow issue,

Overflow code

strcpy(saun.sun_path, sockFile);

There isn't any check if length sockFile greater of length saun.sun_path

For Example

  #include <sys/types.h>
  #include <sys/socket.h>
  #include <sys/un.h>
  #include <stdio.h>

  #define NAME "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"


  main()
  {
      int sock, msgsock, rval;
      struct sockaddr_un server;
      char buf[1024];


      sock = socket(AF_UNIX, SOCK_STREAM, 0);
      if (sock < 0) {
         perror("opening stream socket");
          exit(1);
      }
      server.sun_family = AF_UNIX;
      strcpy(server.sun_path, NAME);
      if (bind(sock, (struct sockaddr *) &server, sizeof(struct sockaddr_un))) {
          perror("binding stream socket");
          exit(1);
      }
      printf("Socket has name %s\n", server.sun_path);
      listen(sock, 5);
      for (;;) {
          msgsock = accept(sock, 0, 0);
          if (msgsock == -1)
              perror("accept");
          else do {
              bzero(buf, sizeof(buf));
              if ((rval = read(msgsock, buf, 1024)) < 0)
                  perror("reading stream message");
              else if (rval == 0)
                  printf("Ending connection\n");
              else
                  printf("-->%s\n", buf);
          } while (rval > 0);
          close(msgsock);
      }
      close(sock);
      unlink(NAME);
}

Output :

=================================================================
==13823==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffed626352e at pc 0x7f8c43cddead bp 0x7ffed6263480 sp 0x7ffed6262c28
WRITE of size 201 at 0x7ffed626352e thread T0
    #0 0x7f8c43cddeac  (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x3feac)
    #1 0x55f40cb24385 in main (/home/kia/codes/C/server+0x1385)
    #2 0x7f8c43ad809a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)
    #3 0x55f40cb24199 in _start (/home/kia/codes/C/server+0x1199)

Address 0x7ffed626352e is located in stack of thread T0 at offset 142 in frame
    #0 0x55f40cb24264 in main (/home/kia/codes/C/server+0x1264)

  This frame has 2 object(s):
    [32, 142) 'server'
    [192, 1216) 'buf' <== Memory access at offset 142 partially underflows this variable
HINT: this may be a false positive if your program uses some custom stack unwind mechanism or swapcontext
      (longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x3feac) 
Shadow bytes around the buggy address:
  0x10005ac44650: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10005ac44660: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10005ac44670: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10005ac44680: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10005ac44690: 00 00 00 00 f1 f1 f1 f1 00 00 00 00 00 00 00 00
=>0x10005ac446a0: 00 00 00 00 00[06]f2 f2 f2 f2 f2 f2 00 00 00 00
  0x10005ac446b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10005ac446c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10005ac446d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10005ac446e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10005ac446f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==13823==ABORTING

Fix

snprintf(saun.sun_path, sizeof(saun.sun_path), "%s", sockFile);

Thanks,

@bk138
Copy link
Member

bk138 commented Apr 2, 2019

Thanks for reporting! Do you have a pull request at hand?

@javadiahmad
Copy link
Author

Yeah, Bug discover by @raminfp from Saminray Security Researcher Team (SSRT),

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants