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

Non-Interactive Execution of android::action:onReceive:intentUrl Opens Large Attack Surface and Should be Optional #227

Closed
bibor opened this issue Apr 13, 2022 · 11 comments · Fixed by #315

Comments

@bibor
Copy link

bibor commented Apr 13, 2022

Current Behaviour

If a sender uses android::action:onReceive:intentUrl, the receiving android application, as documented, executes this intent on the device, without user interaction, if the application is in foreground.

Example message from the docs:

{
  "extras": {
    "android::action": {
      "onReceive": { "intentUrl": "https://gotify.net" }
    }
  }
}

What happens technically, is that a new Intent is generated, the Intent data is set to URI-parsed sender input, and startActivity is called on the newly generated intent.

Code taken from WebSocketService.java

        Intent intent;

        String intentUrl =
                Extras.getNestedValue(
                        String.class, extras, "android::action", "onReceive", "intentUrl");

        if (intentUrl != null) {
            intent = new Intent(Intent.ACTION_VIEW);
            intent.setData(Uri.parse(intentUrl));
            intent.setFlags(Intent.FLAG_ACTIVITY_NEW_TASK);
            startActivity(intent);
        }

Why This is a Problem

I would argue that this is insecure behaviour. It enables an remote actor, who is able to send messages on the specific application, to execute (URL-based) intents, which can trigger various actions on the device, which might not be in favor of the user. This opens an huge attack surface on the receiving device, amplified by the fact that the execution does not require user interaction, if the application is in foreground.

Proposed Solution

The introducing issue #92 and PRs #98 and #102 line out, that this feature is important for some users, so it should be kept. But i would suggest that it should be an optional feature, that the user need to actively turn on in the settings, which is currently, at least to my observations, not the case. The standard beahaviour should be, that no intent is executed without user interaction. I would further propose that every action but just displaying the message should be and remain optional for reducing the attack surface.

@bibor bibor changed the title Uninteractive execution of android::action:onReceive:intentUrl opens huge attack surface and should be optional Non-Interactive Execution of android::action:onReceive:intentUrl Opens Large Attack Surface and Should be Optional Apr 13, 2022
@jmattheis
Copy link
Member

Sounds reasonable, I'm open to accept PRs for this.

@bibor
Copy link
Author

bibor commented Apr 14, 2022

I'll try, when I have time, but it will take a while, since I'm not an android nor a java developer. If someone experienced wants to help it'll be appreciated.

@cyb3rko
Copy link
Contributor

cyb3rko commented Feb 23, 2023

From a security perspective that makes absolute sense.
I will have a look at how to implement it.

@jmattheis What do you think, should we make it the default to show a confirmation dialog?
But you then could switch it off in the settings if you need to.

@jmattheis
Copy link
Member

@cyb3rko Yeah, a confirmation dialog sounds fine. If that's not easily possible, then I'd be okay with having this functionality opt-in, so it's disabled per default, and you can enable it inside the settings.

@cyb3rko
Copy link
Contributor

cyb3rko commented Jul 2, 2023

I wanted to start implementing it, but thinking about the implementation I found a few problems we have to solve.

  1. When your device is locked the dialog can not be shown and the intent is lost.
  2. When you dismiss the dialog and want to open the intent later, there's not way you can do that.

Some thoughts about this:

  • Should we additionally implement a toggle in the settings just like the author mentioned?
  • To include an interactive part we could show a dialog when clicking the notification or the item in the MessagesActivity.
  • We could also show an additional button in the item in the MessagesActivity on messages, which contain the action extra.

What do you think?

@jmattheis
Copy link
Member

We could also show an additional button in the item in the MessagesActivity on messages, which contain the action extra.

No, for that a separate features request exists: gotify/server#494

To include an interactive part we could show a dialog when clicking the notification or the item in the MessagesActivity.

No, that would work against the specification of the onReceive feature. As defined, the intend should be executed when receiving the message, not when clicking the notification.

Should we additionally implement a toggle in the settings just like the author mentioned?

I'd prefer having only one fix for this issue. I'm probably also okay with removing this onReceive feature completely because I don't think there are many use-cases for it.

@cyb3rko
Copy link
Contributor

cyb3rko commented Jul 2, 2023

So you would only go with the settings toggle?

@jmattheis
Copy link
Member

jmattheis commented Jul 3, 2023

Yes, or only the dialog like you proposed. Only the dialog is probably better, because then we can tell the user what the receive action does.

@cyb3rko
Copy link
Contributor

cyb3rko commented Jul 3, 2023

Then we have the mentioned problem:
What should happen, if your phone is locked?

@jmattheis
Copy link
Member

What should happen, if your phone is locked?

Nothing I guess, currently the functionality only works if the gotify app is in focus. So, nothing happens, if gotify is running in the background.

I'm not sure if you can show a dialog if the app isn't in focus. So the only real fix would be a button on the notification, but that's the other ticket I've linked.

@cyb3rko
Copy link
Contributor

cyb3rko commented Jul 4, 2023

Showing a dialog without focus is not possible.
But I think you can start DialogActivities, that would be one way to solve that.

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

Successfully merging a pull request may close this issue.

3 participants