Skip to content

[SAST] Several potential memory leaks #2760

Description

@Canned-pineapple-8

Running static analysis on UWSGi identified several potential memory leaks that I believe are confirmed. They are relatively minor, but still could be fixed.

The findings are:

  1. Memory leak in event_queue_ack_file_monitor
    File: /core/event.c

Problem:
Variable bie is dynamically allocated here:

uwsgi/core/event.c

Lines 1164 to 1167 in 8d116f7

if (isize > sizeof(struct inotify_event)) {
bie = uwsgi_malloc(isize);
rlen = read(id, bie, isize);
}

If the read call fails (line 1173), the code jumps to return NULL without freeing previously allocated bie:

uwsgi/core/event.c

Lines 1173 to 1175 in 8d116f7

if (rlen < 0) {
uwsgi_error("read()");
}

This results in a memory leak.

Suggested patch:
Free bie before returning on read error, but only if it was heap-allocated (i.e., when isize > sizeof(struct inotify_event)):

if (rlen < 0) {
	uwsgi_error("read()");
	
	if (isize > sizeof(struct inotify_event))
		free(bie);
	
	return NULL;
}
  1. Memory leak in uwsgi_attach_fd
    File: /core/io.c

Problem:
Variable id is dynamically allocated here:

uwsgi/core/io.c

Lines 643 to 648 in 8d116f7

if (code && code_len > 0) {
// allocate space for code and num_sockets
id = uwsgi_malloc(code_len + sizeof(int));
memset(id, 0, code_len);
iov.iov_len = code_len + sizeof(int);
}

If the recvmsg fails, the functions returns without freeing id:

uwsgi/core/io.c

Lines 665 to 670 in 8d116f7

len = recvmsg(fd, &msg, 0);
if (len <= 0) {
uwsgi_error("recvmsg()");
free(msg_control);
return NULL;
}

Suggested patch:
Introduce a cleanup block that frees id when appropriate, and use goto cleanup for all error paths.

First, initialize ret to NULL to ensure correct return value on errors:

int *ret = NULL;

Suggested cleanup block before return:

cleanup:
	free(msg_control);
	if (code && code_len > 0) {
		free(id);
	}

	return ret;

Jump there in case of any error, e.g. for recvmsg:

len = recvmsg(fd, &msg, 0);
if (len <= 0) {
	uwsgi_error("recvmsg()");
	goto cleanup;
}
  1. Memory leak in uwsgi_legion_register
    File: /core/legion.c

Problem:
Pointer secret points to dynamically allocated memory when ((unsigned int) cipher_len > s_len):

uwsgi/core/legion.c

Lines 1092 to 1097 in 8d116f7

if ((unsigned int) cipher_len > s_len) {
char *secret_tmp = uwsgi_malloc(cipher_len);
memcpy(secret_tmp, secret, s_len);
memset(secret_tmp + s_len, 0, cipher_len - s_len);
secret = secret_tmp;
}

Similarly, pointer iv points to dynamically allocated memory when (unsigned int) iv_len > s_iv_len:

uwsgi/core/legion.c

Lines 1104 to 1109 in 8d116f7

if ((unsigned int) iv_len > s_iv_len) {
char *secret_tmp = uwsgi_malloc(iv_len);
memcpy(secret_tmp, iv, s_iv_len);
memset(secret_tmp + s_iv_len, '0', iv_len - s_iv_len);
iv = secret_tmp;
}

If the function succeeds and reaches line 1154, it returns without freeing secret or iv, causing memory leaks.

Suggested patch:
Free both pointers before returning if they were heap-allocated:

uwsgi_legion_add(ul);

if ((unsigned int) cipher_len > s_len)
	free(secret);

if ((unsigned int) iv_len > s_iv_len)
	free(iv);

return ul;
  1. Memory leak in parse_sys_envs
    File: /core/utils.c

Problem:
Variable earg is malloc'ed on line 1639:

earg = uwsgi_malloc(strlen(*uenvs + 6) + 1);

If the strchr call fails, the function breaks out of the loop without freeing earg, resulting in a memory leak:

uwsgi/core/utils.c

Lines 1641 to 1644 in 8d116f7

eq_pos = strchr(earg, '=');
if (!eq_pos) {
break;
}

Suggested patch:
Free earg before breaking:

if (!eq_pos) {
	free(earg);
	break;
}
  1. Memory leak in uwsgi_load_plugin
    File: /core/plugins.c

Problem:
Variable plugin_entry_symbol is allocated dynamically here:

uwsgi/core/plugins.c

Lines 174 to 175 in 8d116f7

char *plugin_entry_symbol = uwsgi_concat2n(plugin_symbol_name_start, strlen(plugin_symbol_name_start) - 3, "", 0);
up = dlsym(plugin_handle, plugin_entry_symbol);

If both dlsym() calls fail, the function ends without freeing plugin_entry_symbol:

uwsgi/core/plugins.c

Lines 268 to 274 in 8d116f7

end:
if (need_free)
free(plugin_name);
if (plugin_filename)
free(plugin_filename);
return NULL;

Suggested patch:
Free plugin_entry_symbol before leaving its scope (within the same block where it is allocated):

		if (!has_option)
			uwsgi_log("%s\n", dlerror());
        
        free(plugin_entry_symbol);
	}

end:
	if (need_free)
		free(plugin_name);

Please feel free to get back to me and reject any of the findings described above if you believe they are false positives, or discuss any of my suggested patches. I will adjust the linked PR accordingly.

Thank you for your time and consideration.

Found by Linux Verification Center with SVACE

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions