[Issue]A double free bug in ulfius.c

[ShangzhiXu]

Describe the issue I found a double free bug in ulfius.c, in function internal_ulfius_init_instance

To Reproduce I found it by static analysis~

Expected behavior In internal_ulfius_init_instance from line 1688 to 1703, it looks like this:

 u_map_init(u_instance->default_headers);
    u_instance->default_endpoint = NULL;
    u_instance->max_post_param_size = 0;
    u_instance->max_post_body_size = 0;
    u_instance->file_upload_callback = NULL;
    u_instance->file_upload_cls = NULL;
#ifndef U_DISABLE_GNUTLS
    u_instance->use_client_cert_auth = 0;
#endif
#ifndef U_DISABLE_WEBSOCKET
    u_instance->websocket_handler = o_malloc(sizeof(struct _websocket_handler));
    if (u_instance->websocket_handler == NULL) {
      y_log_message(Y_LOG_LEVEL_ERROR, "Ulfius - Error allocating memory for u_instance->websocket_handler");
      ulfius_clean_instance(u_instance);
      return U_ERROR_MEMORY;
    }

In the first line above, it callsu_map_init(u_instance->default_headers), in this function at src/n_map.c line 45, ifu_instance->default_headers->values fails to alloc, u_instance->default_headers->keys will be freed.

However, in the third line from the bottom above, ulfius_clean_instance(u_instance)will be called, and the u_instance->default_headers->keys will be released again at src/n_map.c line 72

System (please complete the following information):

• OS/Environment [Ubuntu]
• Ulfius Version [2.7.10]/Git master
• Source installation [ github package/build from source]

[babelouest]

Thanks,

this should be fixed in https://github.com/babelouest/ulfius/commit/e458353b56d94b6657467118af40b0466f343b30 , can you check it again?

[ShangzhiXu]

Hi, thanks for your reply! But I think it has not been fixed completely, maybe I didn't explain the issue clearly, sorry for that : )

What I think of the issue is: At line 1689 in ulfius.c, u_map_init(u_instance->default_headers); will be called, in this function,u_instance->default_headers->keys might be freed [u_map.c, line 45].

Then, at line 1701 in ulfius.c, there is a possibility thatulfius_clean_instance(u_instance); will be called. This function, it will execute this trace:

u_map_clean_full(u_instance->default_headers)            [ulfius.c, line 1627] -> 
u_map_clean(u_map)                                       [u_map.c, line 82] -> 
o_free(u_map->keys)                                      [u_map.c, line 72]

In this case, u_instance->default_headers->keys will be freed again. May be it can be fixed by checking if u_map->keys == NULL before [u_map.c, line 72].

Have a good day~

[babelouest]

OK, I think I see now, I've added some u_map->keys = NULL; and al. in the functions u_map_clean and u_map_init in the last commit https://github.com/babelouest/ulfius/commit/0d4f7ab0de321c55a60d29b2c732509f9f605ebd , can you verify it again please?

[ShangzhiXu]

I think it is fine now, so I'll close this issue, thanks for your cooperation!