atheme / libmowgli-2

mowgli development framework (version 2) -- generic runtime for atheme applications
Other
149 stars 31 forks source link

There is a memory leak defect at line 110 in /libmowgli-2/src/libmowgli/base/argstack.c. #53

Open LuMingYinDetect opened 7 months ago

LuMingYinDetect commented 7 months ago

Hello! I am a graduate student specializing in static analysis. Recently, I used a static analysis tool to test libmowgli-2 and found a potential memory leak defect. Here are the specific details of the defect:

Affected Version: libmowgli-2 2.1.3 21abd51006891ba89785cf198e130828453515ea

Vulnerability Description: The vulnerability is a memory leak bug located at line 110 of the file /libmowgli-2/src/libmowgli/base/argstack.c. This vulnerability could potentially be exploited maliciously to cause resource exhaustion and denial of service attacks.

libmowgli-2 download address: https://github.com/atheme/libmowgli-2.git

Detailed Description of the Defect:

1.At line 87 of the file /libmowgli-2/src/libmowgli/base/argstack.c, a pointer named 'e' is defined, and it is allocated a dynamic memory area using the mowgli_alloc function. When the switch statement at line 89 enters the default branch at line 107, although the dynamic memory area stored in the variable 'out' is released by calling the mowgli_object_unref function at line 108, the statement that adds the pointer 'e' to the variable 'out' is located at line 113. This means that in this iteration of the loop, 'e' has not yet been added to the variable 'out'. Subsequently, the program returns at line 110, during which the memory area pointed to by 'e' is not released, thus resulting in a memory leak defect, as illustrated below:

image

ilbelkyr commented 7 months ago

Thank you for the report! However, I don't think this constitutes a security vulnerability.

mowgli_argstack_create_from_va_list provides functionality to parse a varargs list (va_list) into a mowgli_argstack_t data structure. To do so, it needs information about the number and types of arguments expected in the list; this is provided by the caller via the descstr parameter.

Callers are expected to provide accurate information about the argument list; compare this example (using mowgli_formatter_format, which calls mowgli_argstack_create_from_va_list on its varargs list with the provided descstr):

https://github.com/atheme/libmowgli-2/blob/21abd51006891ba89785cf198e130828453515ea/src/examples/formattertest/formattertest.c#L41

If a bug in a calling function allows an attacker to provide an invalid descstr value, they could most likely also change the types or number of parameters specified, causing undefined behaviour upon trying to parse the va_list (or later, once code attempts to work with a mowgli_argstack_t that has been created incorrectly). Due to the nature of varargs in C, we cannot avoid relying on the caller having passed a correct description.

Calling mowgli_argstack_create_from_va_list with a descstr containing characters other than s, d, p and b is definitely an error and should never happen, however; if the default branch of the switch statement you've pointed out is reached, this indicates something has gone very wrong.

I'd argue we should be more strict in this case: if there is a codepath that results in an invalid descstr being passed, this indicates a bug in the caller that might cause other bad descriptions to be passed as well which would cause undefined behaviour without us being able to detect it. (Either way, the caller won't accomplish what it's trying to accomplish.)

cc @aaronmdjones – should we consider calling abort or similar in this case after generating a warning?