medic / cht-android

A native Android container for Community Health Toolkit (CHT) applications
GNU Affero General Public License v3.0
25 stars 50 forks source link

282 - Fix Implicit Intent Vulnerability #283

Closed latin-panda closed 2 years ago

latin-panda commented 2 years ago

Description

This PR:

Ticket: https://github.com/medic/cht-android/issues/282

License

The software is provided under AGPL-3.0. Contributions to this project are accepted under the same license.

latin-panda commented 2 years ago

@garethbowen In theory, it should be enough by just setting the package name, the intent will only be accessible inside the package and fixes the concerns in the "vulnerability" criteria. I found another 2 intents that could be considered as implicit, I fixed those as well, I tested that they still work fine.

I couldn't find a lint option to catch this, so I opened a question in stackoverflow.

  1. Should this be ATed?
  2. Is this going to be released as production or beta in Play Store?
  3. Would this be a v1.0.1?
  4. Do we want to wait for this work and release everything together?

Thanks

garethbowen commented 2 years ago

@latin-panda Have you managed to verify that this change still functions the way we expect? It was added as part of https://github.com/medic/cht-android/issues/163 so it should be possible to try and recreate the issue.

Should this be ATed?

Yes, to make sure it hasn't regressed.

Is this going to be released as production or beta in Play Store?

I think now that we have the production track going we should keep it going.

Would this be a v1.0.1?

I think that'd be good, yes. We've fixed a bug without changing functionality so a service pack bump is appropriate.

Do we want to wait for https://github.com/medic/cht-android/pull/281 and release everything together?

If they line up perfectly that's fine, but otherwise each should be released as soon as possible.

latin-panda commented 2 years ago

@garethbowen Yes, I've tested the areas that changed and they are working fine. Once approved, I'll move the ticket to AT and notify QA for second pair of eyes, before starting the minor release.

latin-panda commented 2 years ago

@garethbowen since this is a patch release, do we want to still make release notes and update CHANGELOG.md? Just like bigger release process

garethbowen commented 2 years ago

@latin-panda Yes, it's best to update the changelog for every release.