babelouest / ulfius

Web Framework to build REST APIs, Webservices or any HTTP endpoint in C language. Can stream large amount of data, integrate JSON data with Jansson, and create websocket services
https://babelouest.github.io/ulfius
GNU Lesser General Public License v2.1
1.08k stars 182 forks source link

[Issue] ulfius_url_{encode,decode} call malloc instad of o_malloc #206

Closed laf0rge closed 2 years ago

laf0rge commented 2 years ago

Describe the issue

All ulfius versions since commit 36c802a936e714504cdbd5bd2388474735551119 (i.e. v2.6.0 ... v2.7.6) are crashing our application software (osmo-remsim-server, see https://osmocom.org/projects/osmo-remsim/wiki and https://git.osmocom.org/osmo-remsim). In all the Osmocom software we are using the libtalloc memory allocator, as it helps significantly with debugging memory leaks. For years, we are using orcania's support for hooking in custom memory allocators, which is also explicitly documented as a feature in API.md of ulfius.

However, since the above mentioned commit, ulfius started to directly call malloc() in url_decode, ulfius_url_decode and ulfius_uil_encode rather than using the o_malloc wrapper. This means that the memory is allocated using libc malloc, but later on then handed to talloc_free (via o_free) -> boom. You cannot allocate with one allocator and free with the other.

To Reproduce You could build + start osmo-remsim-server and then issue a HTTP request against the built-in ulfius based REST server. This will result in the backtrace that is seen at https://osmocom.org/issues/5343

Expected behavior The expected behavior is that all ulfius internal allocations will use o_malloc() and then o_free().

The only way to prevent these kind of bugs or regressions is to use a malloc/free wrapper in the entire ulfius test suite which would immediately detect if ever some pointer is passed to o_free which has not previously been seen at o_malloc.

System (please complete the following information):

laf0rge commented 2 years ago

I'm currently testing a related fix

babelouest commented 2 years ago

Hello @laf0rge ,

Thanks for noticing, and providing the pull request, I think it's worth a new release!

laf0rge commented 2 years ago

thanks for the quick reaction. As I mentioned, I think the only way to avoid such regressions is to alsways use some wrapper during development/testing, whch would ASSERT that any memory passed to o_free() has previously been returned by o_malloc() or o_realloc().