hyperrealm / libconfig

C/C++ library for processing configuration files
https://hyperrealm.github.io/libconfig/
GNU Lesser General Public License v2.1
1.1k stars 359 forks source link

A null pointer reference problem exists in function config_clear(). #225

Closed ghost closed 1 year ago

ghost commented 1 year ago

https://github.com/hyperrealm/libconfig/blob/master/lib/util.h#L26:

define __new(T) (T )calloc(1, sizeof(T)) / zeroed */

calloc() function may fail and NULL is returned.

https://github.com/hyperrealm/libconfig/blob/master/lib/libconfig.c#L739

config->root = __new(config_setting_t); config->root->type = CONFIG_TYPE_GROUP; config->root->config = config;

The value of config->root may be NULL, which poses security risks.

ghost commented 1 year ago

Currently, I do not know how to reproduce the problem. The current problem is detected by using a tool.

thomastrapp commented 1 year ago

Thank you for reporting this. Can you name the tool, that was used to detect this error?

The return value of all memory allocations should be checked for null. Glancing over the code, I can't find any null checks regarding memory allocations.

For example, config_clear, config_setting_create and many others should check for allocation failure and abort (i.e. cleanup and return NULL or CONFIG_FALSE).

ghost commented 1 year ago

I am very sorry for this, the tool is not open source and can't be downloaded from the Internet.

ghost commented 1 year ago

I think using a fuzzing test tool like AFL should also detect these problems.

ghost commented 1 year ago

In actual applications, I think calloc() can hardly fail to apply for memory at https://github.com/hyperrealm/libconfig/blob/master/lib/libconfig.c#L739, and the amount of memory it applies for can be calculated. However, it is necessary to check for NULL and process it for safety purposes.

ghost commented 1 year ago

Hello, are there any plans to fix this bug?

xiaoge1001 commented 1 year ago

@thomastrapp Hello, can you review this patch? I think it can fix this bug.

https://github.com/hyperrealm/libconfig/pull/226

hyperrealm commented 1 year ago

We could certainly check the return value of all malloc/calloc/realloc calls in libconfig. The problem is, there are also memory allocations being done in the code generated by Flex and Bison. What do we do if a memory allocation fails deep inside the lexer or parser? The Flex code by default just writes a message to stderr and calls exit(). You can provide an alternate handler by redefining YY_FATAL_ERROR, but that handler must terminate the process, because the calling code does not expect it to return...it just goes ahead and uses the NULL pointer. So if the handler does return, undefined behavior results. I suppose the handler could do a longjmp() to unwind the stack and try to recover, but at that point you've got partially-allocated parser data structures which may not be possible to clean up, etc...

Best option IMHO would be to add a libconfig_set_fatal_error_handler() function to libconfig, with a default handler that just calls abort().

xiaoge1001 commented 1 year ago

So are there any plans to fix the problem? I don't know much about libconfig. According to the above reply, I can't make a good patch in a short time.

xiaoge1001 commented 1 year ago

I use the tool mentioned at the beginning to detect the problem. And after applying #226 , the problem disappeared.

hyperrealm commented 1 year ago

I'll make a fix next week when I have some days off from work.

xiaoge1001 commented 1 year ago

OK, thanks, look forward to your patch.

xiaoge1001 commented 1 year ago

I found that hyperrealm has fixed this bug: https://github.com/hyperrealm/libconfig/commit/2c40b58062abcec834187ad965e30ad7568ce9b7

Thank you very much.

xiaoge1001 commented 1 year ago

I'm using a linux OS, so do I need the patches that fix the windows build failure? I don't think it's necessary, but if I use these patches, I don't know if there's any problem?

xiaoge1001 commented 1 year ago

I use libconfig-1.7.3, gcc-10.3, automake 1.16.5, libconfig build success. but, when I use the patch command to apply 2c40b58062abcec834187ad965e30ad7568ce9b7, the libconfig build fails. The error information is as follows: [ 299s] /home/abuild/rpmbuild/BUILD/libconfig-1.7.3/aux-build/missing: line 81: aclocal-1.15: command not found [ 299s] WARNING: 'aclocal-1.15' is missing on your system.

After several tests, I found that the problem was caused by the following modification: / Identify the host operating system as Minix. This macro does not affect the system headers' behavior. A future release of Autoconf may stop defining this macro. /

ifndef _MINIX

# undef _MINIX

endif

hyperrealm commented 1 year ago

You probably forgot to regenerate your 'configure' script?

xiaoge1001 commented 1 year ago

Sorry, I did not regenerate the configure script. I run "autoreconf -vfi" to regenerate the configure script, libconfig is successfully built.

xiaoge1001 commented 1 year ago

You can close the current issue. I have deregistered the account that submitted the issue. That account is not commonly used. Currently, I can't close it.