rovo89 / XposedBridge

The Java part of the Xposed framework.
3.9k stars 1.1k forks source link

Security features #21

Open M1cha opened 10 years ago

M1cha commented 10 years ago

I think it might be a good idea to make using xposed more secure. First thing would be a permissions system(we could easily use custom android permissions for that), so not every module has access to every app.

Then we should extend the API that far that most modules don't need to use initZygote anymore(so they won't gain root).

rovo89 commented 10 years ago

I see where you're coming from and how this might be useful.

It's not as easy as it sounds though. Currently, all modules are loaded once when the phone starts. Even if initZygote() isn't called, there are other ways to execute code at that point (not recommended, but possible). So those modules would have to be loaded later, each time the target app is started. Possible, but requires some work to keep an app => modules mapping, loading it etc.

Much more work is required to reflect this in the UI. There would be modules which get access to all apps, and others which are limited to a few of them. Those would have to be displayed so that the user can make his choice (e.g. hooking into the Superuser app is dangerous, too). You would want to confirm this again after an update, but only if there are new target apps. And so on.

I'm not sure what you mean with "extend the API that far that most modules don't need to use initZygote anymore" - which kind of APIs would you like to see? I'm not so confident that "most" modules could be limited to a few non-critical target apps.

Also not sure how and why you would like to use Android permissions here. They can't be used during the execution of module code, because then the hooked app would need these permissions, not the module. They could only be used in the installer, which would then create the module => apps mapping based on it. But there's a catch: Android permissions need to be declared first before they can be used. There are some security risks with that and it won't work if you install the module before Xposed Installer. That's why I'm using metadata - which would be required here anyway to specify the list of target apps the module wants to access.

M1cha commented 10 years ago

Which ways are there to execute code as root? Enable/Disable modules at runtime would be another feature I've already thought about but that's another story.

I'd like to use android permission because the user would see those on installation. And if they've changed he'll see that, too. This way the user can use the UI he's used too and we don't need to copy that feature.

xposed then would check this permissions manually. This is possible since these permissions don't need any special linux permissions for the app process. We just need to check which API calls are allowed and which aren't. Also people could handle these permissions with XPrivacy if we don't use our own way.

"extend the API that far that most modules don't need to use initZygote anymore" I checked some apps which use this function. And the main purpose is the access to the app's sharedpreferences from the custom settings UI of the module.

rovo89 commented 10 years ago

Which ways are there to execute code as root?

For a module? They could put something in their (static) constructor for example.

Enable/Disable modules at runtime would be another feature I've already thought about but that's another story.

Once a module is loaded, it cannot be unloaded. Most modules place their hooks during startup, so enabling them later wouldn't work anyway. Best option is to require a restart.

I'd like to use android permission because the user would see those on installation.

Do you want to have a generic "access only non-critical apps" permission (whatever "non-critical" means)? Or a per-target-app permission? I don't think the latter would be possible, as permissions need to be declared by their exact name before they are referenced. Otherwise, Android won't display them and won't grant them either.

We just need to checks which API calls are allowed and which aren't.

Restrictions for "API calls" could be circumvented and would therefore give users a wrong feeling of security. And they couldn't be enforced correctly because Xposed can't determine which module the request came from. Android services check the UID from a different process. This can't work here.

Checking permissions also requires access to the PackageManager, which isn't running early during startup and requires a Context later, which is created after the handleLoadPackage() calls.

I checked some apps which use this function. And the main purpose is the access to the app's sharedpreferences from the custom settings UI of the module.

Then these modules are doing something wrong. You don't need Xposed APIs and root access for that. You just have to ensure that the shared preferences XML is stored with world-readable permissions. Only when you expect that a permission "fix" script could have been executed in recovery, it can be useful to use XSharedPreferences.makeWorldReadable() to fix the permission. It's not at all a necessity though, especially as such scripts are used less these days.

By the way, I have grepped through 364 of the modules in the repository. I haven't checked in detail, but 213 of them seem to implement initZygote(). I'm definitely not gonna break compatibility with those, so the default (at least apps targetting the current Xposed API version) would be to allow everything.

rovo89 commented 10 years ago

Nevertheless - making it possible to write modules with limited scope would be nice. It's quite complex and needs lots of thoughts though, so it's a mid- to long-term enhancement.

M1cha commented 10 years ago

"For a module? They could put something in their (static) constructor for example." wow I didn't even think about this. I dunno if there's a way to fix this. afaik setuid doesn't allow to go back to root.

"Once a module is loaded, it cannot be unloaded. Most modules place their hooks during startup, so enabling them later wouldn't work anyway. Best option is to require a restart." if they don't need to hijack system apps this shouldn't be a problem because every app has it's own process. I understand why it's not possible with the current code structure though. this is off topic here anyway.

"Do you want to have a generic "access only non-critical apps" permission (whatever "non-critical" means)? Or a per-target-app permission? I don't think the latter would be possible, as permissions need to be declared by their exact name before they are referenced. Otherwise, Android won't display them and won't grant them either." I meant per-target-app permissions. I thought it's possible because apps can define their own permissions. Would be some hacky string parsing though. Probably not the best idea.

"Restrictions for "API calls" could be circumvented and would therefore give users a wrong feeling of security. And they couldn't be enforced correctly because Xposed can't determine which module the request came from. Android services check the UID from a different process. This can't work here.

Checking permissions also requires access to the PackageManager, which isn't running early during startup and requires a Context later, which is created after the handleLoadPackage() calls." Just to make sure, you mean xposed api's not android/framework, right?. If so it sounds like the the xposedbridge is just a helper and the user could use their own implementation? I mean once the module has code running in the target app's process we lost all control, but we don't need per class/method control anyway. xposed just shouldn't call the Hook methods of modules which aren't allowed to hook app X.

About the sharedpreferences: they use initZygote to set these permissions. A xposed api to say "hey I need access to the database of the app I'm allowed to hijcack" would be a good idea

about the compatibility: ofc we shouldn't break it and introduce this features in newer api versions only.

rovo89 commented 10 years ago

Please prefix quotes with ">", makes it easier to read. ;) You can also simply mark the text and press "r".

wow I didn't even think about this. I dunno if there's a way to fix this. afaik setuid doesn't allow to go back to root.

Not sure what exactly you mean, but my point was: The module's classes must not be loaded in the Zygote process at all, otherwise they might get root access. Preventing the initZygote() call is not enough.

I thought it's possible because apps can define their own permissions.

Yes, but they need to do that. Any requested permissions that aren't defined by some app are just ignored, even if some app defines it later.

Just to make sure, you mean xposed api's not android/framework, right?.

Yes, I meant restricting calls to Xposed's API.

If so it sounds like the the xposedbridge is just a helper and the user could use their own implementation? I mean once the module has code running in the target app's process we lost all control, but we don't need per class/method control anyway. xposed just shouldn't call the Hook methods of modules which aren't allowed to hook app X.

It's some kind of helper, yes, but the main point is indeed that any module which can run some code in a certain process could use that to open itself more doors. It could use reflection to bypass some restrictions, it could even load a native library to modify the same VM structures that Xposed modifies. I don't see how Xposed could know which module a callback belongs to. Modules melt with the app, I don't think there is a reliable way to tell them apart.

About the sharedpreferences: they use initZygote to set these permissions. A xposed api to say "hey I need access to the database of the app I'm allowed to hijcack" would be a good idea

You of course have access to any files that the hooked app owns. You become a part of the app and can interact with all its permissions.

What you probably mean is your module's files, which the hooked app cannot access by default. But as I said: This isn't the right way to go anyway. As soon as you save the settings, Android will reset the permissions again, unless you use MODE_WORLD_READABLE like this: https://github.com/rovo89/PlayStoreFixes/blob/master/src/de/robv/android/xposed/mods/playstorefix/SettingsActivity.java#L29 If you do that, you don't need any Xposed APIs to access the file.

M1cha commented 10 years ago

Not sure what exactly you mean, but my point was: The module's classes must not be loaded in the Zygote process at all, otherwise they might get root access. Preventing the initZygote() call is not enough.

Yes, I meant restricting calls to Xposed's API.

It's some kind of helper, yes, but the main point is indeed that any module which can run some code in a certain process could use that to open itself more doors. It could use reflection to bypass some restrictions, it could even load a native library to modify the same VM structures that Xposed modifies. I don't see how Xposed could know which module a callback belongs to. Modules melt with the app, I don't think there is a reliable way to tell them apart.

That shouldn't be a problem because we only want to restrict into which app's modules are allowed to be loaded. Anything else won't make sense because even without xposed apps can modify their own process and hook methods using NDK.

Yes, but they need to do that. Any requested permissions that aren't defined by some app are just ignored, even if some app defines it later.

and there's no way for xposed to see these ignored permissions?

rovo89 commented 10 years ago

and there's no way for xposed to see these ignored permissions?

It could parse the manifest. But that would defeat the purpose because the user never gets to see and accept these permissions.

M1cha commented 10 years ago

so our only real problem is how to set/read/show the permissions, right? It's sad that we can't use android permissions for that but we can still write our own.

We could show the permissions window when the user activate a module the first time. And if the app claims more permissions after a update it should be disabled until the user confirmed them.

rovo89 commented 10 years ago

so our only real problem is how to set/read/show the permissions, right?

Well, "problem"... I think this feature is doable, it's mainly a question of effort required to implement it in a good way. And of course module authors would need to adopt it where they can.

M1cha commented 10 years ago

yes and ofc we need to check why exactly so many modules need to be loaded in zygote otherwise all apps will add the zygote/root permission and the whole security system is useless.

rovo89 commented 10 years ago

Well, some might not need it. Some provide system-wide functionality. And if you want to replace resources that could be referenced by other processes (e.g. launcher icons), then your module has to run in these processes as well.

M1cha commented 10 years ago

but the system framework(with the resources u meant, package name "android") doesn't run with root permission, right?

rovo89 commented 10 years ago

You mean the system_server process, where all the system services run? No, it runs with UID=1000 I think. Allowing a module to hook something in there would be almost as dangerous as in the Zygote process though, because it could easily disable permission checks and much more.

Note that this process doesn't have much to do with the system resources. They can be used by any process.

M1cha commented 10 years ago

btw if we add a permission for loading native code we could restrict the xposed api, too or am I missing sth?

rovo89 commented 10 years ago

Sorry, I don't understand what you mean. "loading native code" = System.loadLibrary() and so on? XPrivacy can restrict that, but it will obviously crash an app which needs native libraries. I don't think the framework should get into this business. "restrict the xposed api" => Which API functions exactly? How would they be restricted if no new native libraries could be loaded?

M1cha commented 10 years ago

well it was related our previous discussion - that once the module got loaded into a app's process we can't check what it's doing.

I agree that restricting System.loadLibrary() for module classes only would be a little bit too hacky for a framework like xposed though.

M1cha commented 10 years ago

"restrict the xposed api" => Which API functions exactly? How would they be restricted if

I mean't class/method hook's such a permission system would be way too detailed though.

rovo89 commented 10 years ago

restricting System.loadLibrary() for module classes only

I don't think this would even be possible. It would require parsing the call stack to find out who the caller is, and there would certainly be ways to obfuscate it, like using reflection. I think it's better to keep warning people that modules can run as root and in every app, instead of adding security functions with holes (which would make people less careful, because they wrongly assume that everything is safe). I believe that modules can only be restricted to certain apps safely if they are not loaded at all in other processes, so that should be the way to go.

XspeedPL commented 8 years ago

If modules would need to declare the packages they hook somewhere, you could display that list to the user, in the Installer for example. But then you'd need to make sure modules can't actually hook anything outside handleLoadPackage or something.