[V0.10] Coverity fixes

common/os_calls.c

@@ -1805,7 +1805,7 @@ g_set_wait_obj(tintptr obj)
         return 0;
     }
     fd = obj >> 16;
-    to_write = 4;
+    to_write = sizeof(buf);
     written = 0;
     while (written < to_write)
     {
@@ -1823,12 +1823,13 @@ g_set_wait_obj(tintptr obj)
                 return 1;
             }
         }
-        else if (error > 0)
+        else if (error > 0 && error <= (int)sizeof(buf))
         {
             written += error;
         }
         else
         {
+            // Shouldn't get here.
             return 1;
         }
     }
@@ -2311,15 +2312,27 @@ g_file_set_cloexec(int fd, int status)
 struct list *
 g_get_open_fds(int min, int max)
 {
+    if (min < 0)
+    {
+        min = 0;
+    }
+
     struct list *result = list_create();
 
     if (result != NULL)
     {
         if (max < 0)
         {
-            max = sysconf(_SC_OPEN_MAX);
+            // sysconf() returns a long. Limit it to a sane value
+#define SANE_MAX 100000
+            long sc_max = sysconf(_SC_OPEN_MAX);
+            max = (sc_max < 0) ? 0 :
+                  (sc_max > (long)SANE_MAX) ? SANE_MAX :
+                  sc_max;
+#undef SANE_MAX
         }
 
+        // max and min are now both guaranteed to be >= 0
         if (max > min)
         {
             struct pollfd *fds = g_new0(struct pollfd, max - min);

common/string_calls.c

@@ -444,6 +444,7 @@ g_atoix(const char *str)
         str += 2;
         base = 16;
     }
+    //coverity[OVERRUN:FALSE]
     return strtol(str, NULL, base);
 }
 
@@ -1190,7 +1191,7 @@ utf8_get_next_char(const char **utf8str_ref, unsigned int *len_ref)
     /*
      * Macro used to parse a continuation character
      * @param cp Character Pointer (incremented on success)
-     * @param end One character past end of input string
+     * @param end One character past end of input string, or NULL
      * @param value The value we're constructing
      * @param finish_label Where to go in the event of an error */
 #define PARSE_CONTINUATION_CHARACTER(cp, end, value, finish_label) \
@@ -1209,7 +1210,7 @@ utf8_get_next_char(const char **utf8str_ref, unsigned int *len_ref)
 
     /* Easier to work with unsigned chars and no indirection */
     const unsigned char *cp = (const unsigned char *)*utf8str_ref;
-    const unsigned char *end = (len_ref != NULL) ? cp + *len_ref : cp + 6;
+    const unsigned char *end = (len_ref != NULL) ? cp + *len_ref : NULL;
 
     if (cp == end)
     {

libipm/libipm_recv.c

@@ -135,7 +135,7 @@ libipm_msg_in_check_available(struct trans *trans, int *available)
 enum libipm_status
 libipm_msg_in_wait_available(struct trans *trans)
 {
-    tbus wobj[1];
+    tbus wobj[2]; // trans_get_wait_objs() can return at most 2 elements
     int ocnt = 0;
     enum libipm_status rv = E_LI_SUCCESS;
 

neutrinordp/xrdp-neutrinordp.c

@@ -1790,14 +1790,18 @@ lfreerdp_synchronize(rdpContext *context)
 static boolean
 lfreerdp_pre_connect(freerdp *instance)
 {
+#define MAX_FREERDP_CHANNELS \
+    (sizeof(instance->settings->channels) / \
+     sizeof(instance->settings->channels[0]))
+
     struct mod *mod;
     int index;
     int error;
     int num_chans;
     int target_chan;
     int ch_flags;
     char ch_name[256];
-    const char *ch_names[MAX_STATIC_CHANNELS];
+    const char *ch_names[MAX_FREERDP_CHANNELS];
     char *dst_ch_name;
 
     LOG_DEVEL(LOG_LEVEL_INFO, "lfreerdp_pre_connect:");
@@ -1828,7 +1832,7 @@ lfreerdp_pre_connect(freerdp *instance)
                 LOG(LOG_LEVEL_INFO, "Channel '%s' not passed to module",
                     ch_name);
             }
-            else if (target_chan < MAX_STATIC_CHANNELS)
+            else if (target_chan < MAX_FREERDP_CHANNELS)
             {
                 dst_ch_name = instance->settings->channels[target_chan].name;
                 ch_names[target_chan] = dst_ch_name;

neutrinordp/xrdp-neutrinordp.h

@@ -229,7 +229,7 @@ struct mod
     struct bitmap_item bitmap_cache[4][4096];
     struct brush_item brush_cache[64];
     struct pointer_item pointer_cache[32];
-    char pamusername[255];
+    char pamusername[256];
 
     int allow_client_experiencesettings;
     int perf_settings_override_mask; /* Performance bits overridden in ini file */

sesman/chansrv/smartcard_pcsc.c

@@ -731,6 +731,7 @@ scard_function_list_readers_return(void *user_data,
     struct trans *con;
     struct pcsc_list_readers *pcscListReaders;
     char *msz_readers = NULL;
+    int rv;
 
     LOG_DEVEL(LOG_LEVEL_DEBUG, "scard_function_list_readers_return:");
     LOG_DEVEL(LOG_LEVEL_DEBUG, "  status 0x%8.8x", status);
@@ -787,37 +788,41 @@ scard_function_list_readers_return(void *user_data,
     out_s = trans_get_out_s(con, 8192);
     if (out_s == NULL)
     {
-        return 1;
+        rv = 1;
     }
-    s_push_layer(out_s, iso_hdr, 8);
-    out_uint32_le(out_s, llen);
-    out_uint32_le(out_s, readers);
+    else
     {
-        const char *p = msz_readers;
-        for (index = 0; index < readers; index++)
+        s_push_layer(out_s, iso_hdr, 8);
+        out_uint32_le(out_s, llen);
+        out_uint32_le(out_s, readers);
         {
-            unsigned int slen = strlen(p);
-            if (slen < 100)
-            {
-                out_uint8a(out_s, p, slen);
-                out_uint8s(out_s, 100 - slen);
-            }
-            else
+            const char *p = msz_readers;
+            for (index = 0; index < readers; index++)
             {
-                out_uint8a(out_s, p, 99);
-                out_uint8s(out_s, 1);
+                unsigned int slen = strlen(p);
+                if (slen < 100)
+                {
+                    out_uint8a(out_s, p, slen);
+                    out_uint8s(out_s, 100 - slen);
+                }
+                else
+                {
+                    out_uint8a(out_s, p, 99);
+                    out_uint8s(out_s, 1);
+                }
+                p += (slen + 1);
             }
-            p += (slen + 1);
         }
+        out_uint32_le(out_s, status); /* SCARD_S_SUCCESS status */
+        s_mark_end(out_s);
+        bytes = (int) (out_s->end - out_s->data);
+        s_pop_layer(out_s, iso_hdr);
+        out_uint32_le(out_s, bytes - 8);
+        out_uint32_le(out_s, 0x03); /* SCARD_LIST_READERS 0x03 */
+        rv = trans_force_write(con);
     }
     free(msz_readers);
-    out_uint32_le(out_s, status); /* SCARD_S_SUCCESS status */
-    s_mark_end(out_s);
-    bytes = (int) (out_s->end - out_s->data);
-    s_pop_layer(out_s, iso_hdr);
-    out_uint32_le(out_s, bytes - 8);
-    out_uint32_le(out_s, 0x03); /* SCARD_LIST_READERS 0x03 */
-    return trans_force_write(con);
+    return rv;
 }
 
 /*****************************************************************************/

sesman/sesexec/session.c

@@ -495,9 +495,6 @@ start_x_server(struct login_info *login_info,
                 unknown_session_type = 1;
         }
 
-        g_free(passwd_file);
-        passwd_file = NULL;
-
         if (xserver_params == NULL)
         {
             LOG(LOG_LEVEL_ERROR, "Out of memory allocating X server params");
@@ -520,6 +517,7 @@ start_x_server(struct login_info *login_info,
     }
 
     /* should not get here */
+    g_free(passwd_file);
     list_delete(xserver_params);
     LOG(LOG_LEVEL_ERROR, "A fatal error has occurred attempting "
         "to start the X server on display %u, aborting connection",

xrdp/xrdp_wm.c

@@ -1260,7 +1260,7 @@ xrdp_wm_clear_popup(struct xrdp_wm *self)
     //struct xrdp_bitmap* b;
 
     //b = 0;
-    if (self->popup_wnd != 0)
+    if (self->popup_wnd != NULL)
     {
         //b = self->popup_wnd->popped_from;
         i = list_index_of(self->screen->child_list, (long)self->popup_wnd);
@@ -1269,6 +1269,7 @@ xrdp_wm_clear_popup(struct xrdp_wm *self)
                  self->popup_wnd->width, self->popup_wnd->height);
         xrdp_bitmap_invalidate(self->screen, &rect);
         xrdp_bitmap_delete(self->popup_wnd);
+        self->popup_wnd = NULL;
     }
 
     //xrdp_wm_set_focused(self, b->parent);