search

ifm / xmlrpc-c

:zap: Inofficial fork of the http://xmlrpc-c.sourceforge.net/ library with CMake support
3 stars 4 forks source link

Possible pointer dereference #7

Open Bbulatov opened 11 months ago

Bbulatov commented 11 months ago

Good day! During the static analysis, it was revealed that in some places in the code situations arise that lead to various errors: 1) Dereference after free. In src/method.c, the listSignatures() function calls parseOneSignature() on 218, which frees the memory of the SignatureP variable, but then SignaturePP is assigned the value of SignatureP, which has already been cleared. Next, in the listSignatures() function, this variable is accessed at 225. The same situation is observed in the file src/registry.c (freeing up memory at 115, accessed at 123), src/xmlrpc_client.c (freeing up memory at 1039, accessed at 1047), src/xmlrpc_decompose.c(freeing up memory at 1080, access on 1083), lib/curl_transport/xmlrpc_curl_transport.c(freeing up memory at 1431, access on 1436), lib/curl_transport/xmlrpc_curl_transport.c (freeing up memory at 1552, access on 1559) 2) Double frees can occur in src/xmlrpc_parse.c. In the xmlrpc_parse_call() function, parseCallXml() is called, where xml_element_free() is called, and then the same function can be called again if you go further through the code in the xmlrpc_parse_call() function. 3) A null pointer dereference can occur in src/xmlrpc_server_cgi.c in the function xmlrpc_server_cgi_process_call() in the condition on line 210. When (!type) is executed, the type is called in the condition block, but it does not exist. Does this require change? Could this lead to something critical?

graugans commented 11 months ago

Hej @Bbulatov,

thank you very much for the details. What tools have you used? What is the use-case you have in mind?

Bbulatov commented 11 months ago

@graugans, these errors can lead to an abnormal termination. Errors were identified using the Svace static analyzer. Testing was carried out on Linux Debian OS

Bbulatov commented 11 months ago

I apologize, there was confusion, in the second point, of course, there is no double free, but also dereference after free. In src/xmlrpc_parse.c the xmlrpc_parse_call() function calls parseCallXml() on 308, which frees the memory of the callElemP variable, but then callElemPP is assigned the value of callElemP, which has already been cleared.

graugans commented 11 months ago

Okay, we primarily forked this repository for the sake of the CMake integration, because the original author has not supported CMake. We only use the client part so I do not see much benefit in fixing the server part which is number 3 in your list.

It looks like you are already deep into this, could you please create a pull request to solve this?

Bbulatov commented 18 hours ago

The developer agreed with point 3 and made the following commit: https://sourceforge.net/p/xmlrpc-c/code/3241/