CESNET / libnetconf2

C NETCONF library
BSD 3-Clause "New" or "Revised" License
203 stars 147 forks source link

Segfault when sending malformed XML #488

Closed awesomenode closed 6 months ago

awesomenode commented 6 months ago

Hi!

Using the latest libnetconf2/libyang, I accidentally sent a malformed XML to the server, and it caused a segfault.

I modified one of the libnetconf2 examples, and attached the faulty get-config filter, which caused the issue. Passing this filter with netopeer-cli to the example server should reproduce the issue. malformed_xml_segfault.zip

Thanks!

michalvasko commented 6 months ago

Please provide the exact steps to reproduce. Usually it is difficult to send invalid messages because libnetconf2 checks XML validity on the client side, too, and that is the issue I have. But you said you encountered this while using libnetconf2, so I guess it is possible somehow.

awesomenode commented 6 months ago

I replaced the attached server.c file with the server.c under examples (in the libnetconf2 repo). I built the project, and started the server under build/examples/server in one window.

Meanwhile in an other window I started netopeer-cli:

> connect --ssh --host 127.0.0.1 --port 10000 --login admin admin@127.0.0.1 password: > get-config --source running --filter-subtree=get_config.xml nc ERROR: SSH channel poll error (Socket error: disconnected). cli_send_recv: Failed to receive a reply.

michalvasko commented 6 months ago

Um, the issue is in the code you added, LYD_TREE_DFS_BEGIN() uses different parameters than LYD_TREE_DFS_END(), they must always be the same. I am a bit confused.

awesomenode commented 6 months ago

Sorry I messed up. Here are the updated files: malformed_xml_segfault.zip

The difference is, that in netopeer-cli I used:

> user-rpc --content rpc.xml

michalvasko commented 6 months ago

The problem is still in your code. You cannot simply take tree value of an anydata node. You must first check the type of the stored value, that is why it is a union.

struct lyd_node_any *any;
switch (any->value_type) {
case LYD_ANYDATA_DATATREE:
    // any->value.tree;
case LYD_ANYDATA_STRING:
    // any->value.str;
case LYD_ANYDATA_XML:
    // any->value.xml;
case LYD_ANYDATA_JSON:
    // any->value.json;
case LYD_ANYDATA_LYB:
    // any->value.mem;
}
awesomenode commented 6 months ago

My bad then. For some reason I just assumed filter will always be LYD_ANYDATA_DATATREE. Thanks for clearing that up!

michalvasko commented 6 months ago

It will, as long as it is valid.