Some examples obscure the problems instead of securing them

[domenukk]

First of all, I think this repo is a great idea as teaching security will be a very important topic going forward.
I want to thank the authors for the work they put into this, a lot of time and effort clearly has gone into crafting those examples.

The only thing I noticed is that some examples might need discussion concerning the actual security benefit of the given solutions:

Insecure Activity Access:
The vulnerability presented here is that an attacker can launch the second activity directly.
The fix is, basically, setting a fixed key, kind of like a password.
Not only would this be very prone to brute force in the example (numerical, so 5000 tries on average), but the attacker can also download the apk, decompile it and read the password.
Instead, it might be more important to teach not to export Activities by accident in the manifest. Modern Androids won't allow access to them if they are not exported. (If an attacker roots the phone, all is lost anyway).
Instead of the key, a static variable that's shared between both activities, indicating the login status, could also be an alternative - depending on the thread model.

Content Provider:
In this example, the content provider is basically encrypting the data with a pre-shared key. While the crypography in this scenario is not really meant to be secure, even the likes of aes would not help - for the simple reason that an attacker can decompile the app and figure out the mechanism. We can only ever add obscurity to the process this way, not security.
Instead, depending on the use case/attacker, we should probably solve the problem with permissions, just as discussed in the broadcast readme. For example we could restrict the content to our own signed apps only.

The same basic problem can be seen in Data Storage. The data is encrypted with a key that could simply be read from the binary. Of course, this problem cannot be solved, conceptually. Not even WhatApp manges to do it... An attacker with access to the apps' private data will always be able to decrypt the contents. It might, of course, still be a good idea to encrypt data to make the target a little bit harder to attack - however then we might want to add a proper discussion about the implications.

Service
The secured service checks if callingActivity.equals("MyAuthorizedApp"). While it's nice to see different methods that can be used for security, an attacker can simply name his activity that way as well. Just as in the broadcast example, I think this problem might better be solved using permissions, depending on the attack scenario.

This issue is intended to start a positive discussion and to develop the repository further. While I understand the target audience will mostly be beginners, I feel it's still important to have proper best practices in place or, at least, discuss certain drawbacks with them. Else they might accidentaly introduce security bugs after all.

I would like to see this project succeed. Thank you again for your efforts.