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

use MD5 will cause integrate failed. #250

Closed
lsors opened this issue Jun 1, 2020 · 6 comments · Fixed by #456
Closed

use MD5 will cause integrate failed. #250

lsors opened this issue Jun 1, 2020 · 6 comments · Fixed by #456
Assignees
Labels
bug Something isn't working

Comments

@lsors
Copy link

lsors commented Jun 1, 2020

call CC_MD5 in md5HexDigest, when we integrate Amplitude, scanning tool report this defect, it easy to fix, but if have this defect , our app can not integrate Amplitude for security concern. hope you fix it .

@haoliu-amp
Copy link
Contributor

Thanks, we will look into it.

@haoliu-amp
Copy link
Contributor

This crypto algorithm is used for our checksum field, actually you don't need to worry about the security concern for that. However, we will see if we wanna switch it to SHA256.

@lsors
Copy link
Author

lsors commented Jul 28, 2020

great, we very much hope it switch to SHA256 . I know this crypto algorithm is not for security use, but scan report tool will still show it in report , some customer will have security concern for that, explain it will also have poor effective ,and competitor will also make waves in market

@JanNash
Copy link
Contributor

JanNash commented Aug 30, 2020

Hey @haoliu-amp, is there an update on a possible switch to SHA256? I'd gladly submit a PR to update the CC_MD5 usage to SHA256 in this framework. I'd also gladly fix the other two deprecation warnings in said PR.

JanNash added a commit to JanNash/Amplitude-iOS that referenced this issue Oct 19, 2020
As mentioned by @haoliu-amp in
amplitude#250 (comment),
> This crypto algorithm is used for our checksum field, actually you don't need to worry about the security concern for that.
> However, we will see if we wanna switch it to SHA256.
Based on this, we can silence the compile warning here until a fix is implemented.
@haoliu-amp
Copy link
Contributor

@JanNash Thanks for your help contributing and I need to work with our backend team to make changes there to coordinate with this change. However, you can submit a PR for sure.

Thanks again.

JanNash added a commit to JanNash/Amplitude-iOS that referenced this issue Oct 19, 2020
As mentioned by @haoliu-amp in
amplitude#250 (comment),
> This crypto algorithm is used for our checksum field, actually you don't need to worry about the security concern for that.
> However, we will see if we wanna switch it to SHA256.
Based on this, we can silence the compile warning here until a fix is implemented.
JanNash added a commit to JanNash/Amplitude-iOS that referenced this issue Oct 19, 2020
As mentioned by @haoliu-amp in
amplitude#250 (comment),
> This crypto algorithm is used for our checksum field, actually you don't need to worry about the security concern for that.
> However, we will see if we wanna switch it to SHA256.
Based on this, we can silence the compile warning here until a fix is implemented.
@JanNash
Copy link
Contributor

JanNash commented Oct 19, 2020

Thanks again.

No biggie :)

haoliu-amp pushed a commit that referenced this issue Oct 20, 2020
* fix(deprecations): Add `+ (UIWindow *)getKeyWindow;` wrapper to AMPUtils

* fix(deprecations): Add diagnostic ignored around `statusBarFrame` usage

* fix(deprecations): Add availability-checked unarchive method to Amplitude

* fix(deprecations): Add availability checking to archive method in Amplitude

* fix(deprecations): Wrap usage of `CC_MD5` into `diagnostic ignored`

As mentioned by @haoliu-amp in
#250 (comment),
> This crypto algorithm is used for our checksum field, actually you don't need to worry about the security concern for that.
> However, we will see if we wanna switch it to SHA256.
Based on this, we can silence the compile warning here until a fix is implemented.

* fix(deprecations): Add tvOS to availability checks

* style: Add missing newlines, replace dot syntax by bracket syntax
github-actions bot pushed a commit that referenced this issue Oct 20, 2020
## [7.1.1](v7.1.0...v7.1.1) (2020-10-20)

### Bug Fixes

* **buildsettings:** Remove override for GCC_WARN_INHIBIT_ALL_WARNINGS ([#302](#302)) ([0e55297](0e55297))
* **deprecation warnings:** Fix deprecation warnings ([#301](#301)) ([e7b0e6e](e7b0e6e)), closes [/github.com//issues/250#issuecomment-655224554](https://github.com//github.com/amplitude/Amplitude-iOS/issues/250/issues/issuecomment-655224554)
* **deprecations:** Use DEPRECATED_MSG_ATTRIBUTE instead of notes ([#305](#305)) ([f501c6c](f501c6c))
* nil dynamic config refresh crash ([#288](#288)) ([#289](#289)) ([9dc896d](9dc896d))
* Swift UserId and DeviceId setter ([#299](#299)) ([b7c0f90](b7c0f90))
@jooohhn jooohhn added bug Something isn't working and removed type: bug labels Nov 4, 2020
@justin-fiedler justin-fiedler self-assigned this Jun 30, 2023
@justin-fiedler justin-fiedler linked a pull request Jun 30, 2023 that will close this issue
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants