Skip to content

Commit

Permalink
Only pass the RFCOMM port's integer value...
Browse files Browse the repository at this point in the history
...to blueman-mechanism

This avoids arbitrary files getting sent to blueman-mechanism to see the results for privileged access (i.e. it can be used to detect arbitrary files and directories).
  • Loading branch information
cschramm committed Feb 8, 2022
1 parent 09176ab commit 14a2ae3
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 12 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@

* Dropped the PIN database

### Bugs fixed

* blueman-mechanism accepted arbitrary file paths and returned the errors from trying to open them, see https://github.com/blueman-project/blueman/security/advisories/GHSA-3r9p-m5c8-8mw8

## 2.2.3

### Bugs fixed
Expand Down
14 changes: 7 additions & 7 deletions blueman/plugins/applet/PPPSupport.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ def on_ppp_connected(self, device: Device, rfcomm: str, ppp_port: str) -> None:


class Connection:
def __init__(self, applet: "BluemanApplet", service: DialupNetwork, port: str,
def __init__(self, applet: "BluemanApplet", service: DialupNetwork, port: int,
ok: Callable[[str], None], err: Callable[[GLib.Error], None]):
self.reply_handler = ok
self.error_handler = err
Expand All @@ -48,26 +48,26 @@ def connect(self) -> bool:
c = Config("org.blueman.gsmsetting", f"/org/blueman/gsmsettings/{self.service.device['Address']}/")

m = Mechanism()
m.PPPConnect('(sss)', self.port, c["number"], c["apn"], result_handler=self.on_connected,
m.PPPConnect('(uss)', self.port, c["number"], c["apn"], result_handler=self.on_connected,
error_handler=self.on_error)

return False

def on_error(self, _obj: Mechanism, result: GLib.Error, _user_data: None) -> None:
logging.info(f"Failed {result}")
# FIXME confusingly self.port is the full rfcomm device path but the service expects the number only
self.error_handler(result)

def _connect() -> bool:
self.service.disconnect(int(self.port[-1]))
self.service.disconnect(self.port)
return False

GLib.timeout_add(1000, _connect)

def on_connected(self, _obj: Mechanism, result: str, _user_data: None) -> None:
self.reply_handler(self.port)
rfcomm_dev = f"/dev/rfcomm{self.port:d}"
self.reply_handler(rfcomm_dev)
for plugin in self.parent.Plugins.get_loaded_plugins(PPPConnectedListener):
plugin.on_ppp_connected(self.service.device, self.port, result)
plugin.on_ppp_connected(self.service.device, rfcomm_dev, result)

msg = _("Successfully connected to <b>DUN</b> service on <b>%(0)s.</b>\n"
"Network is now available through <b>%(1)s</b>") % {"0": self.service.device['Alias'], "1": result}
Expand All @@ -85,7 +85,7 @@ class PPPSupport(AppletPlugin, RFCOMMConnectHandler):
def rfcomm_connect_handler(self, service: SerialService, reply: Callable[[str], None],
err: Callable[[Union[RFCOMMError, GLib.Error]], None]) -> bool:
if isinstance(service, DialupNetwork):
def local_reply(port: str) -> None:
def local_reply(port: int) -> None:
assert isinstance(service, DialupNetwork) # https://github.com/python/mypy/issues/2608
Connection(self.parent, service, port, reply, err)

Expand Down
6 changes: 3 additions & 3 deletions blueman/plugins/mechanism/Ppp.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

class Ppp(MechanismPlugin):
def on_load(self) -> None:
self.parent.add_method("PPPConnect", ("s", "s", "s"), "s", self._ppp_connect, pass_sender=True, is_async=True)
self.parent.add_method("PPPConnect", ("u", "s", "s"), "s", self._ppp_connect, pass_sender=True, is_async=True)

def _ppp_connected(self, _ppp: PPPConnection, port: str, ok: Callable[[str], None]) -> None:
ok(port)
Expand All @@ -16,12 +16,12 @@ def _ppp_error(self, _ppp: PPPConnection, message: str, err: Callable[[str], Non
err(message)
self.timer.resume()

def _ppp_connect(self, port: str, number: str, apn: str, caller: str,
def _ppp_connect(self, port: int, number: str, apn: str, caller: str,
ok: Callable[[str], None], err: Callable[[str], None]) -> None:
self.confirm_authorization(caller, "org.blueman.pppd.pppconnect")
self.timer.stop()

ppp = PPPConnection(port, number, apn)
ppp = PPPConnection(f"/dev/rfcomm{port:d}", number, apn)
ppp.connect("error-occurred", self._ppp_error, err)
ppp.connect("connected", self._ppp_connected, ok)

Expand Down
4 changes: 2 additions & 2 deletions blueman/services/meta/SerialService.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ def try_replace_root_watcher(self, monitor: Gio.FileMonitor, path: str, port: in

def connect(
self,
reply_handler: Optional[Callable[[str], None]] = None,
reply_handler: Optional[Callable[[int], None]] = None,
error_handler: Optional[Callable[[RFCOMMError], None]] = None
) -> bool:
# We expect this service to have a reserved UUID
Expand All @@ -96,7 +96,7 @@ def connect(
self.try_replace_root_watcher(mon, filename, port_id)

if reply_handler:
reply_handler(filename)
reply_handler(port_id)
except RFCOMMError as e:
if error_handler:
error_handler(e)
Expand Down

0 comments on commit 14a2ae3

Please sign in to comment.