[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2890?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Alexander A. Strelets updated ZOOKEEPER-2890:
---------------------------------------------
    Description: 
Function *_deserialize_response()_*, in _case COMPLETION_STRING_, uses local 
automatic variable *_struct CreateResponse res_* which is +left uninitialized+ 
and passed to the function _deserialize_GetACLResponse()_ and then to 
_deallocate_GetACLResponse()_.

The _deserialize_ function, which is called the first, is expected to assign 
the _res_ variable with a value from the parsed _struct iarchive *ia_. But, if 
_ia_ contains for example insufficient amount of bytes the 
_deserialize_String()_ function refuses of assigning a value to _res_, and 
_res_ stays uninitialized (the true case is described below). Then, the 
_deallocate_ function calls _deallocate_String()_ passing uninitialized _res_ 
with arguments. If incidentally the memory region in the program stack under 
the _res_ was not equal to NULL, the last call +leads to _free()_ by invalid 
address+.

The true case: this happens when an active _multi_ request with _create_ 
sub-request is completed on call to _zookeeper_close()_ with the so called 
"Fake response" which is fabricated by the function _free_completions()_. Such 
response includes only the header but +zero bytes for the body+. The 
significant condition is that the _create_ request is not a stand-alone one, 
but namely a sub-request within the _multi_ request. In this case the 
_deserialize_response()_ is called recursively (for each sub-request), and when 
it is called for the _create_ subrequest (from the nested 
_deserialize_multi()_) the _failed_ parameter is assigned with false (0), so 
the _if (failed)_ condition branches to the _else_ part. Note that in the 
stand-alone create-request case this does not occur.

*I suspect this may happen not only due to call to _zookeeper_close()_ but on 
reception of a true multi-response from the server* containing insufficient 
number of bytes (I'm not sure if it can be a proper response from the server 
with an error overall status and empty or insufficient payload).

  was:
Function *_deserialize_response()_*, in _case COMPLETION_STRING_, uses local 
automatic variable *_struct CreateResponse res_* which is +left uninitialized+ 
and passed to the function _deserialize_GetACLResponse()_ and then to 
_deallocate_GetACLResponse()_.

The _deserialize_ function, which is called the first, is expected to assign 
the _res_ variable with a value from the parsed _struct iarchive *ia_. But, if 
_ia_ contains for example insufficient amount of bytes the 
_deserialize_String()_ function refuses of assigning a value to _res_, and 
_res_ stays uninitialized (the true case is described below). Then, the 
_deallocate_ function calls _deallocate_String()_ passing uninitialized _res_ 
with arguments. If incidentally the memory region in the program stack under 
the _res_ was not equal to NULL, the last call +leads to _free()_ for an 
invalid address+.

The true case: this happens for example when an active _create_ request (or 
create sub-request within the _multi_ request) is completed on call to 
_zookeeper_close()_ with the so called "Fake response" which is fabricated by 
the function _free_completions()_. Such response includes only the header but 
+zero bytes for the body+.

*I suspect this may happen not only due to call to _zookeeper_close()_ but on 
reception of a true response from the server* containing insufficient number of 
bytes (I'm not sure if it can be a proper response from the server with an 
error status and empty payload).

Also *I suspect the same case will take place with different requests, but not 
only the _create_*. Indeed, almost all cases in the _deserialize_response()_ 
shall be verified as soon as they also use uninitialized _res_-s and 
_deallocate_ them. Still I have not checked others except the _create_ request 
with _COMPLETION_STRING_ response.


> Local automatic variable is left uninitialized and then freed.
> --------------------------------------------------------------
>
>                 Key: ZOOKEEPER-2890
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2890
>             Project: ZooKeeper
>          Issue Type: Bug
>          Components: c client
>    Affects Versions: 3.4.10
>         Environment: Linux ubuntu 4.4.0-87-generic
> gcc (Ubuntu 5.4.0-6ubuntu1~16.04.4) 5.4.0 20160609
> https://github.com/apache/zookeeper.git
> branch-3.4
>            Reporter: Alexander A. Strelets
>            Priority: Critical
>              Labels: easyfix
>             Fix For: 3.4.10
>
>
> Function *_deserialize_response()_*, in _case COMPLETION_STRING_, uses local 
> automatic variable *_struct CreateResponse res_* which is +left 
> uninitialized+ and passed to the function _deserialize_GetACLResponse()_ and 
> then to _deallocate_GetACLResponse()_.
> The _deserialize_ function, which is called the first, is expected to assign 
> the _res_ variable with a value from the parsed _struct iarchive *ia_. But, 
> if _ia_ contains for example insufficient amount of bytes the 
> _deserialize_String()_ function refuses of assigning a value to _res_, and 
> _res_ stays uninitialized (the true case is described below). Then, the 
> _deallocate_ function calls _deallocate_String()_ passing uninitialized _res_ 
> with arguments. If incidentally the memory region in the program stack under 
> the _res_ was not equal to NULL, the last call +leads to _free()_ by invalid 
> address+.
> The true case: this happens when an active _multi_ request with _create_ 
> sub-request is completed on call to _zookeeper_close()_ with the so called 
> "Fake response" which is fabricated by the function _free_completions()_. 
> Such response includes only the header but +zero bytes for the body+. The 
> significant condition is that the _create_ request is not a stand-alone one, 
> but namely a sub-request within the _multi_ request. In this case the 
> _deserialize_response()_ is called recursively (for each sub-request), and 
> when it is called for the _create_ subrequest (from the nested 
> _deserialize_multi()_) the _failed_ parameter is assigned with false (0), so 
> the _if (failed)_ condition branches to the _else_ part. Note that in the 
> stand-alone create-request case this does not occur.
> *I suspect this may happen not only due to call to _zookeeper_close()_ but on 
> reception of a true multi-response from the server* containing insufficient 
> number of bytes (I'm not sure if it can be a proper response from the server 
> with an error overall status and empty or insufficient payload).



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Reply via email to