[vino/wip/oholy/covescan-fixes: 6/6] Fix various defects reported by covscan



commit af30833fe2c7629ee2102853a4c01b71f56acf43
Author: Ondrej Holy <oholy redhat com>
Date:   Tue Aug 28 14:24:08 2018 +0200

    Fix various defects reported by covscan
    
    This patch fixes the following important defects reported by covscan:
    server/libvncserver/main.c:178: leaked_storage: Variable "i" going out of scope leaks the storage it 
points to.
    server/libvncserver/rfbserver.c:195: leaked_storage: Variable "cl" going out of scope leaks the storage 
it points to.
    server/libvncserver/rfbserver.c:1161: overwrite_var: Overwriting "i" in "i = 
sraRgnGetIterator(updateRegion)" leaks the storage that "i"
    server/libvncserver/rfbserver.c:1232: deref_arg: Calling "sraRgnReleaseIterator" dereferences freed 
pointer "i".
    server/libvncserver/rfbserver.c:1291: leaked_storage: Variable "i" going out of scope leaks the storage 
it points to.
    server/libvncserver/sockets.c:635: leaked_handle: Handle variable "sock" going out of scope leaks the 
handle.
    server/libvncserver/sockets.c:635: leaked_handle: Handle variable "sock6" going out of scope leaks the 
handle.
    server/libvncserver/sockets.c:639: leaked_handle: Handle variable "sock" going out of scope leaks the 
handle.
    server/libvncserver/sockets.c:639: leaked_handle: Handle variable "sock6" going out of scope leaks the 
handle.
    server/libvncserver/sockets.c:663: overwrite_var: Overwriting handle "sock" in "sock = 
NewSocketListenTCP((struct sockaddr *)s4, 16U)" leaks the handle.
    server/libvncserver/sockets.c:677: overwrite_var: Overwriting handle "sock" in "sock = 
NewSocketListenTCP((struct sockaddr *)s6, 46U)" leaks the handle.
    server/libvncserver/sockets.c:691: leaked_handle: Handle variable "sock" going out of scope leaks the 
handle.
    server/libvncserver/tableinit24.c:150:7: warning: this 'if' clause does not guard... 
[-Wmisleading-indentation]
    server/miniupnp/minissdpc.c:43: buffer_size_warning: Calling strncpy with a maximum size argument of 108 
bytes on destination array "addr.sun_path" of size 108 bytes might leave the destination string unterminated.
    server/miniupnp/miniupnpc.c:405: leaked_handle: Handle variable "sudp" going out of scope leaks the 
handle.
    server/smclient/eggsmclient-xsmp.c:1171: missing_va_end: va_end was not called for "ap".

 server/libvncserver/main.c         | 2 ++
 server/libvncserver/rfbserver.c    | 7 +++++++
 server/libvncserver/sockets.c      | 6 +++---
 server/libvncserver/tableinit24.c  | 8 ++++----
 server/miniupnp/minissdpc.c        | 2 +-
 server/miniupnp/miniupnpc.c        | 1 +
 server/smclient/eggsmclient-xsmp.c | 1 +
 7 files changed, 19 insertions(+), 8 deletions(-)
---
diff --git a/server/libvncserver/main.c b/server/libvncserver/main.c
index 016bbff..0d9a737 100644
--- a/server/libvncserver/main.c
+++ b/server/libvncserver/main.c
@@ -175,6 +175,8 @@ void rfbDoCopyRegion(rfbScreenInfoPtr rfbScreen,sraRegionPtr copyRegion,int dx,i
    }
   
    rfbScheduleCopyRegion(rfbScreen,copyRegion,dx,dy);
+
+   sraRgnReleaseIterator(i);
 }
 
 void rfbDoCopyRect(rfbScreenInfoPtr rfbScreen,int x1,int y1,int x2,int y2,int dx,int dy)
diff --git a/server/libvncserver/rfbserver.c b/server/libvncserver/rfbserver.c
index 0a60fb2..a880b53 100644
--- a/server/libvncserver/rfbserver.c
+++ b/server/libvncserver/rfbserver.c
@@ -192,6 +192,7 @@ rfbNewClient(rfbScreenInfoPtr rfbScreen,
       if (fcntl(sock, F_SETFL, O_NONBLOCK) < 0) {
        rfbLogPerror("fcntl failed");
        close(sock);
+       free(cl);
        return NULL;
       }
 #endif
@@ -200,6 +201,7 @@ rfbNewClient(rfbScreenInfoPtr rfbScreen,
                     (char *)&one, sizeof(one)) < 0) {
        rfbLogPerror("setsockopt failed");
        close(sock);
+       free(cl);
        return NULL;
       }
 
@@ -1089,6 +1091,7 @@ rfbSendFramebufferUpdate(rfbClientPtr cl,
             int h = rect.y2 - y;
            nUpdateRegionRects += (((h-1) / (ZLIB_MAX_SIZE( w ) / w)) + 1);
        }
+       sraRgnReleaseIterator(i);
 #ifdef VINO_HAVE_JPEG
     } else if (cl->preferredEncoding == rfbEncodingTight) {
        nUpdateRegionRects = 0;
@@ -1141,6 +1144,8 @@ rfbSendFramebufferUpdate(rfbClientPtr cl,
 
     UNLOCK(cl->cursorMutex);
 
+   i = NULL;
+
    if (sendCursorShape) {
        cl->cursorWasChanged = FALSE;
        if (!rfbSendCursorShape(cl))
@@ -1288,6 +1293,8 @@ rfbSendCopyRegion(rfbClientPtr cl,
 
     }
 
+    sraRgnReleaseIterator(i);
+
     return TRUE;
 }
 
diff --git a/server/libvncserver/sockets.c b/server/libvncserver/sockets.c
index ee755eb..2366e19 100644
--- a/server/libvncserver/sockets.c
+++ b/server/libvncserver/sockets.c
@@ -625,12 +625,12 @@ ListenOnTCPPort(rfbScreenInfoPtr rfbScreen, int port, const char *netIface)
       rfbLog("Listening IPv4://0.0.0.0:%d\n", port);
 
 #ifdef VINO_ENABLE_IPV6
-    if(sock6 > 0) {
+    if(sock6 >= 0) {
        psock[*ptot] = sock6;
       *ptot        += 1;
     }
 #endif
-    if(sock > 0) {
+    if(sock >= 0) {
        psock[*ptot] = sock;
       *ptot        += 1;
     }
@@ -683,7 +683,7 @@ ListenOnTCPPort(rfbScreenInfoPtr rfbScreen, int port, const char *netIface)
     }
 #endif       
 
-    if(sock > 0) {
+    if(sock >= 0) {
        psock[*ptot] = sock;
       *ptot        += 1;
        sock         = -1;
diff --git a/server/libvncserver/tableinit24.c b/server/libvncserver/tableinit24.c
index 39e9920..575a501 100644
--- a/server/libvncserver/tableinit24.c
+++ b/server/libvncserver/tableinit24.c
@@ -149,9 +149,9 @@ rfbInitOneRGBTable24 (uint8_t *table, int inMax, int outMax, int outShift,
       *(uint32_t *)&table[3*i] = outValue;
       if(!rfbEndianTest)
        memmove(table+3*i,table+3*i+1,3);
-        if (swap) {
-         c = table[3*i]; table[3*i] = table[3*i+2];
-         table[3*i+2] = c;
-        }
+      if (swap) {
+       c = table[3*i]; table[3*i] = table[3*i+2];
+       table[3*i+2] = c;
+      }
     }
 }
diff --git a/server/miniupnp/minissdpc.c b/server/miniupnp/minissdpc.c
index d37599f..64772f9 100644
--- a/server/miniupnp/minissdpc.c
+++ b/server/miniupnp/minissdpc.c
@@ -40,7 +40,7 @@ getDevicesFromMiniSSDPD(const char * devtype, const char * socketpath)
                return NULL;
        }
        addr.sun_family = AF_UNIX;
-       strncpy(addr.sun_path, socketpath, sizeof(addr.sun_path));
+       strncpy(addr.sun_path, socketpath, sizeof(addr.sun_path) - 1);
        if(connect(s, (struct sockaddr *)&addr, sizeof(struct sockaddr_un)) < 0)
        {
                /*syslog(LOG_WARNING, "connect(\"%s\"): %m", socketpath);*/
diff --git a/server/miniupnp/miniupnpc.c b/server/miniupnp/miniupnpc.c
index f9d620e..718526a 100644
--- a/server/miniupnp/miniupnpc.c
+++ b/server/miniupnp/miniupnpc.c
@@ -402,6 +402,7 @@ struct UPNPDev * upnpDiscover(int delay, const char * multicastif,
 #endif
        {
                PRINT_SOCKET_ERROR("setsockopt");
+               closesocket(sudp);
                return NULL;
        }
 
diff --git a/server/smclient/eggsmclient-xsmp.c b/server/smclient/eggsmclient-xsmp.c
index d5cf3b5..5ca976f 100644
--- a/server/smclient/eggsmclient-xsmp.c
+++ b/server/smclient/eggsmclient-xsmp.c
@@ -1162,6 +1162,7 @@ array_prop (const char *name, ...)
       pv.value = value;
       g_array_append_val (vals, pv);
     }
+  va_end (ap);
 
   prop->num_vals = vals->len;
   prop->vals = (SmPropValue *)vals->data;


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