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

[Android Tool] Replace Outdated Drozer when Possible #1904

Merged
merged 27 commits into from
Oct 9, 2021
Merged

[Android Tool] Replace Outdated Drozer when Possible #1904

merged 27 commits into from
Oct 9, 2021

Conversation

righettod
Copy link
Member

@righettod righettod commented Apr 19, 2021

PR closes #1839

  • Your contribution is written in the 2nd person (e.g. you)
  • Your contribution is written in an active present form for as much as possible.
  • You have made sure that the reference section is up to date (e.g. please add sources you have used, make sure that the references to MITRE/MASVS/etc. are up to date)
  • Your contribution has proper formatted markdown and/or code
  • Any references to website have been formatted as [TEXT](URL “NAME”)
  • You verified/tested the effectiveness of your contribution (e.g.: is the code really an effective remediation? Please verify it works!)

@righettod
Copy link
Member Author

Failure of check links are not related to the PR 😢

@righettod
Copy link
Member Author

righettod commented Apr 20, 2021

Now Drozer is only present in the following documents and the usage described of it is consistant in terms of usage:

image

I propose to handle these documents in a next PR in order to move, step by step, and keep the guide consistant in terms of tools used to prevent switching inside the same test phase.

@righettod righettod marked this pull request as ready for review April 20, 2021 06:15
@righettod righettod changed the title Replacement of Drozer when possible. Replacement of DROZER when possible. Apr 20, 2021
@righettod righettod changed the title Replacement of DROZER when possible. [ANDROID] Replacement of DROZER when possible. Apr 20, 2021
@righettod righettod changed the title [ANDROID] Replacement of DROZER when possible. [Android] Replacement of DROZER when possible. Apr 20, 2021
@righettod
Copy link
Member Author

righettod commented Apr 21, 2021 via email

Copy link
Collaborator

@cpholguera cpholguera left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First round review done ;) Please also consider the comment I've sent before that is not included in this review. Thanks!

Document/0x05h-Testing-Platform-Interaction.md Outdated Show resolved Hide resolved
Document/0x05h-Testing-Platform-Interaction.md Outdated Show resolved Hide resolved
@righettod
Copy link
Member Author

righettod commented Apr 21, 2021 via email

@cpholguera
Copy link
Collaborator

Awesome! Thanks 😌

@righettod
Copy link
Member Author

@cpholguera : Update pushed in the feature branch and status described here.

@TheDauntless TheDauntless self-requested a review April 28, 2021 11:43
@righettod
Copy link
Member Author

Hi @cpholguera and @TheDauntless
Did I need to fix or change something?
Thanks in advance 😃

@righettod
Copy link
Member Author

Hi @cpholguera and @TheDauntless

Any news?

Thanks a lot in advance 😃

@cpholguera
Copy link
Collaborator

hi @righettod, we were very busy with the new release:

https://github.com/OWASP/owasp-masvs/releases/tag/v1.3

We'll get back to you as soon as we can. Sorry for the wait. Have a great day!

@righettod
Copy link
Member Author

Hi @cpholguera ,
Ha ok I apologize for my message, I was not aware of the coming release.
Congratulations 💯
Do not worry with my PR, take the time you need 😃
Have a nice day too!

@righettod
Copy link
Member Author

Hi @cpholguera and @TheDauntless ,

Any news about this PR?

No stress at all, i'm just asking in case of you are waiting on a action on my side 😃

@cpholguera
Copy link
Collaborator

Hi @righettod we're very sorry that we cannot give any feedback yet. We're still dealing with a lot of things regarding the next big steps for both MASVS and MSTG so we cannot really address any PRs yet. we'll let you know as soon as we manage to go back to normal. Thanks again for your patience!

@righettod
Copy link
Member Author

Hi @cpholguera ,

No stress at all, as said, it was just to know if you are waiting any action on my side.

Take the time you need, no stress at all once again 😃

Thank a lot for all your work on MSTG and MASVS, the team rocks!!!!! ❤️ ❤️ ❤️ ❤️

@TheDauntless
Copy link
Collaborator

We definitely want to replace Drozer wherever possible with adb comments. Drozer is outdated and the added value is super limited.

The one place where I do see added value is when you're trying to attack the system; Using Drozer it's possible to scan which applications have requested a specific permission and which of those applications are system applications. I think that's currently not easily possible through adb comments, but some more research may need to be done here.

We will probably be doing some heavy refactoring in the near future, but we should try to merge your PR before we do that, as it fully aligns with our goals.

@righettod
Copy link
Member Author

@cpholguera @TheDauntless Thank you very much for your feedback.

Copy link
Collaborator

@cpholguera cpholguera left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @righettod, we're very sorry for the delay and hope that you're aware of our situation and everything that's going on in the project. I finally could take some time today to do the final review. If you agree with the suggestions please apply them directly and we can merge ;)

Document/0x05h-Testing-Platform-Interaction.md Outdated Show resolved Hide resolved
Document/0x05h-Testing-Platform-Interaction.md Outdated Show resolved Hide resolved
Comment on lines 324 to +327
When doing the dynamic analysis: validate whether the permission requested by the app is actually necessary for the app. For instance: a single-player game that requires access to `android.permission.WRITE_SMS`, might not be a good idea.

To obtain detail about a specific permission you can refer to the [Android Documentation](https://developer.android.com/reference/android/Manifest.permission "Android Permissions").

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already wrote above about this. In this section we only focus on testing and interpreting results.

Suggested change
When doing the dynamic analysis: validate whether the permission requested by the app is actually necessary for the app. For instance: a single-player game that requires access to `android.permission.WRITE_SMS`, might not be a good idea.
To obtain detail about a specific permission you can refer to the [Android Documentation](https://developer.android.com/reference/android/Manifest.permission "Android Permissions").

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion accepted 😃

Document/0x05h-Testing-Platform-Interaction.md Outdated Show resolved Hide resolved
Document/0x05h-Testing-Platform-Interaction.md Outdated Show resolved Hide resolved
Document/0x05h-Testing-Platform-Interaction.md Outdated Show resolved Hide resolved
Document/0x05h-Testing-Platform-Interaction.md Outdated Show resolved Hide resolved
Document/0x05h-Testing-Platform-Interaction.md Outdated Show resolved Hide resolved
righettod and others added 8 commits October 9, 2021 18:32
Co-authored-by: cpholguera <perezholguera@gmail.com>
Co-authored-by: cpholguera <perezholguera@gmail.com>
Co-authored-by: cpholguera <perezholguera@gmail.com>
Co-authored-by: cpholguera <perezholguera@gmail.com>
Co-authored-by: cpholguera <perezholguera@gmail.com>
Co-authored-by: cpholguera <perezholguera@gmail.com>
Co-authored-by: cpholguera <perezholguera@gmail.com>
Co-authored-by: cpholguera <perezholguera@gmail.com>
@righettod
Copy link
Member Author

righettod commented Oct 9, 2021

@cpholguera You will never need to apologize with me, I know the both sides of the life of an OWASP PL (moreover with a flagship one like the MSTG) and the heavy load implied on spare times. I have accepted and committed all your proposal 😃

Thank a lot again for your amazing work on this project as well as your support on this PR ❤️

Links validation failure does not seem to come from my PR 😢

image

righettod and others added 5 commits October 9, 2021 18:44
Co-authored-by: cpholguera <perezholguera@gmail.com>
Co-authored-by: cpholguera <perezholguera@gmail.com>
Co-authored-by: cpholguera <perezholguera@gmail.com>
Co-authored-by: cpholguera <perezholguera@gmail.com>
Co-authored-by: cpholguera <perezholguera@gmail.com>
Copy link
Collaborator

@cpholguera cpholguera left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the quick response and for the nice words ☺️ It's also an honor to have such great contributors as you. Thanks a lot for your support!

@cpholguera cpholguera merged commit 53ebd2c into OWASP:master Oct 9, 2021
@cpholguera cpholguera changed the title [Android] Replacement of DROZER when possible. [Android] Replace Outdated Drozer Tool when Possible Dec 23, 2021
@cpholguera cpholguera changed the title [Android] Replace Outdated Drozer Tool when Possible [Android Tool] Replace Outdated Drozer when Possible Dec 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Drozer / needle - is it still alive?
3 participants