PDP-10 / supdup

Community maintained SUPDUP client for Unix
Other
18 stars 8 forks source link

Descriptor leaks and use-without-checks #37

Open johnsonjh opened 2 months ago

johnsonjh commented 2 months ago

(These line numbers are with PR #36 applied.)

Generated on Linux with GCC 14.2.1 via env CFLAGS=-fanalyzer CC=gcc make:

gcc -fanalyzer -D_DEFAULT_SOURCE -D_XOPEN_SOURCE=600   -c -o supdup.o supdup.c
gcc -fanalyzer -D_DEFAULT_SOURCE -D_XOPEN_SOURCE=600   -c -o charmap.o charmap.c
gcc -fanalyzer -D_DEFAULT_SOURCE -D_XOPEN_SOURCE=600   -c -o tcp.o tcp.c
gcc -fanalyzer -D_DEFAULT_SOURCE -D_XOPEN_SOURCE=600   -c -o chaos.o chaos.c

chaos.c: In function ‘connect_to_named_socket’:
chaos.c:48:12: warning: leak of file descriptor ‘sock’ [CWE-775] [-Wanalyzer-fd-leak]
   48 |     return -1;
      |            ^
  ‘chaos_connect’: events 1-2
    |
    |  112 | chaos_connect(const char *hostname, const char *contact)
    |      | ^~~~~~~~~~~~~
    |      | |
    |      | (1) entry to ‘chaos_connect’
    |  113 | {
    |  114 |   int fd = connect_to_named_socket(SOCK_STREAM, "chaos_stream");
    |      |            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |            |
    |      |            (2) calling ‘connect_to_named_socket’ from ‘chaos_connect’
    |
    +--> ‘connect_to_named_socket’: events 3-6
           |
           |   30 | connect_to_named_socket(int socktype, char *path)
           |      | ^~~~~~~~~~~~~~~~~~~~~~~
           |      | |
           |      | (3) entry to ‘connect_to_named_socket’
           |......
           |   35 |   if ((sock = socket(AF_UNIX, socktype, 0)) < 0) {
           |      |      ~        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           |      |      |        |
           |      |      |        (4) stream socket created here
           |      |      |        (5) when ‘socket’ succeeds
           |      |      (6) following ‘false’ branch (when ‘sock >= 0’)...
           |
         ‘connect_to_named_socket’: events 7-9
           |
           |   40 |   server.sun_family = AF_UNIX;
           |      |                     ^
           |      |                     |
           |      |                     (7) ...to here
           |......
           |   44 |   if (connect(sock, (struct sockaddr *)&server, slen) < 0) {
           |      |      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           |      |      ||
           |      |      |(8) when ‘connect’ fails
           |      |      (9) following ‘true’ branch...
           |
         ‘connect_to_named_socket’: event 10
           |
           |   45 |     if (errno != ENOENT)
           |      |         ^~~~~
           |      |         |
           |      |         (10) ...to here
           |
         ‘connect_to_named_socket’: event 11
           |
           |   48 |     return -1;
           |      |            ^
           |      |            |
           |      |            (11) ‘sock’ leaks here
           |

chaos.c: In function ‘write_all’:
chaos.c:58:9: warning: ‘write’ on possibly invalid file descriptor ‘fd’ [-Wanalyzer-fd-use-without-check]
   58 |     m = write(fd, x, n);
      |         ^~~~~~~~~~~~~~~
  ‘chaos_connect’: events 1-2
    |
    |  112 | chaos_connect(const char *hostname, const char *contact)
    |      | ^~~~~~~~~~~~~
    |      | |
    |      | (1) entry to ‘chaos_connect’
    |  113 | {
    |  114 |   int fd = connect_to_named_socket(SOCK_STREAM, "chaos_stream");
    |      |            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |            |
    |      |            (2) calling ‘connect_to_named_socket’ from ‘chaos_connect’
    |
    +--> ‘connect_to_named_socket’: events 3-6
           |
           |   30 | connect_to_named_socket(int socktype, char *path)
           |      | ^~~~~~~~~~~~~~~~~~~~~~~
           |      | |
           |      | (3) entry to ‘connect_to_named_socket’
           |......
           |   35 |   if ((sock = socket(AF_UNIX, socktype, 0)) < 0) {
           |      |      ~        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           |      |      |        |
           |      |      |        (4) when ‘socket’ fails
           |      |      (5) following ‘true’ branch (when ‘sock < 0’)...
           |   36 |     perror("socket(AF_UNIX)");
           |      |     ~~~~~~~~~~~~~~~~~~~~~~~~~
           |      |     |
           |      |     (6) ...to here
           |
    <------+
    |
  ‘chaos_connect’: events 7-8
    |
    |  114 |   int fd = connect_to_named_socket(SOCK_STREAM, "chaos_stream");
    |      |            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |            |
    |      |            (7) returning to ‘chaos_connect’ from ‘connect_to_named_socket’
    |......
    |  117 |   if (connection(fd, hostname, contact) < 0) {
    |      |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |       |
    |      |       (8) calling ‘connection’ from ‘chaos_connect’
    |
    +--> ‘connection’: events 9-10
           |
           |   86 | connection(int net, const char *host, const char *contact)
           |      | ^~~~~~~~~~
           |      | |
           |      | (9) entry to ‘connection’
           |......
           |   93 |   if (write_all(net, buf, n) < n)
           |      |       ~~~~~~~~~~~~~~~~~~~~~~
           |      |       |
           |      |       (10) calling ‘write_all’ from ‘connection’
           |
           +--> ‘write_all’: events 11-14
                  |
                  |   53 | static ssize_t write_all(int fd, void *buf, size_t n)
                  |      |                ^~~~~~~~~
                  |      |                |
                  |      |                (11) entry to ‘write_all’
                  |......
                  |   57 |   while (n > 0) {
                  |      |          ~~~~~  
                  |      |            |
                  |      |            (12) following ‘true’ branch (when ‘n != 0’)...
                  |   58 |     m = write(fd, x, n);
                  |      |         ~~~~~~~~~~~~~~~
                  |      |         |
                  |      |         (13) ...to here
                  |      |         (14) ‘fd’ could be invalid
                  |

chaos.c: In function ‘read_all’:
chaos.c:74:9: warning: ‘read’ on possibly invalid file descriptor ‘fd’ [-Wanalyzer-fd-use-without-check]
   74 |     m = read(fd, x, n);
      |         ^~~~~~~~~~~~~~
  ‘chaos_connect’: events 1-2
    |
    |  112 | chaos_connect(const char *hostname, const char *contact)
    |      | ^~~~~~~~~~~~~
    |      | |
    |      | (1) entry to ‘chaos_connect’
    |  113 | {
    |  114 |   int fd = connect_to_named_socket(SOCK_STREAM, "chaos_stream");
    |      |            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |            |
    |      |            (2) calling ‘connect_to_named_socket’ from ‘chaos_connect’
    |
    +--> ‘connect_to_named_socket’: events 3-6
           |
           |   30 | connect_to_named_socket(int socktype, char *path)
           |      | ^~~~~~~~~~~~~~~~~~~~~~~
           |      | |
           |      | (3) entry to ‘connect_to_named_socket’
           |......
           |   35 |   if ((sock = socket(AF_UNIX, socktype, 0)) < 0) {
           |      |      ~        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           |      |      |        |
           |      |      |        (4) when ‘socket’ fails
           |      |      (5) following ‘true’ branch (when ‘sock < 0’)...
           |   36 |     perror("socket(AF_UNIX)");
           |      |     ~~~~~~~~~~~~~~~~~~~~~~~~~
           |      |     |
           |      |     (6) ...to here
           |
    <------+
    |
  ‘chaos_connect’: events 7-8
    |
    |  114 |   int fd = connect_to_named_socket(SOCK_STREAM, "chaos_stream");
    |      |            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |            |
    |      |            (7) returning to ‘chaos_connect’ from ‘connect_to_named_socket’
    |......
    |  117 |   if (connection(fd, hostname, contact) < 0) {
    |      |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |       |
    |      |       (8) calling ‘connection’ from ‘chaos_connect’
    |
    +--> ‘connection’: events 9-12
           |
           |   86 | connection(int net, const char *host, const char *contact)
           |      | ^~~~~~~~~~
           |      | |
           |      | (9) entry to ‘connection’
           |......
           |   93 |   if (write_all(net, buf, n) < n)
           |      |      ~
           |      |      |
           |      |      (10) following ‘false’ branch...
           |......
           |   96 |   bp = buf;
           |      |   ~~~~~~~~
           |      |      |
           |      |      (11) ...to here
           |   97 |   while (read_all(net, cbuf, 1) == 1) {
           |      |          ~~~~~~~~~~~~~~~~~~~~~~
           |      |          |
           |      |          (12) calling ‘read_all’ from ‘connection’
           |
           +--> ‘read_all’: events 13-16
                  |
                  |   69 | static ssize_t read_all(int fd, void *buf, size_t n)
                  |      |                ^~~~~~~~
                  |      |                |
                  |      |                (13) entry to ‘read_all’
                  |......
                  |   73 |   while (n > 0) {
                  |      |          ~~~~~  
                  |      |            |
                  |      |            (14) following ‘true’ branch (when ‘n != 0’)...
                  |   74 |     m = read(fd, x, n);
                  |      |         ~~~~~~~~~~~~~~
                  |      |         |
                  |      |         (15) ...to here
                  |      |         (16) ‘fd’ could be invalid
                  |

gcc  -o supdup supdup.o charmap.o tcp.o chaos.o -lncurses -ltinfo