Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FS#3126 - rpcd crashes upon calling uci add method to add non-existent option with an empty value #7880

Closed
openwrt-bot opened this issue May 26, 2020 · 2 comments
Labels

Comments

@openwrt-bot
Copy link

olegio170:

The root cause of this bug is the same as for one that was fixed in commit:
https://git.openwrt.org/?p=project/rpcd.git;a=commit;h=bd0ed2521476c3e5b6c1a0e0bd2c386ea809d74b

Description
It is possible to crash rpcd using uci add method with values that contain non-existent option with empty value and if section already exists.

**
Steps to reproduce:**

  • Add a new section to some config file e.g firewall. It doesn't matter which values we use or which name. Please note that this step can be omitted and used some existing section. It is provided only to simplify reproduce instruction. ubus call uci add '{"config": "firewall", "name":"unique_name", "type": "rule", "values": {"target":"ACCEPT"}}'
  • Add options to created section, using one non-existent option with empty value. ubus call uci add '{"config": "firewall", "name":"unique_name_1", "type": "rule", "values": {"description": "", "target":"ACCEPT"}}' After that rpcd will crash.

Technical cause:
This bug is caused by the fact that flags in uci_ptr in rpc_uci_add function is not cleaned correctly. As section exists, after calling rpc_uci_lookup to find section UCI_LOOKUP_COMPLETE flag will be set. Later on during handing each provided by user key:value pair, that uci_ptr with UCI_LOOKUP_COMPLETE flag set will be used without cleaning to set new values into config. If provided by user option doesn't exist in config uci_ptr should NOT have UCI_LOOKUP_COMPLETE flag set during the call of uci_set. However, due to the absence of ptr cleaning, even for option from step 2 "description" which does not exist in the config that flag is set. That leads to the cleaning of the whole section instead of option in uci_set function. This leads to use-after-free when we try to set value for the next option and cause rpcd crash.

Proposed fix:
Add cleaning of uci_ptr flags in for_each key:value loop in rpc_uci_add function.
Please see attached patch.

@openwrt-bot
Copy link
Author

olegio170:

Typo in Steps 2 to reproduce: name should be unique_name instead of unique_name_1.
So, full command will be:
ubus call uci add '{"config": "firewall", "name":"unique_name", "type": "rule", "values": {"description": "", "target":"ACCEPT"}}'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant