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

Vulnerability fix for hardcoded encryption key. #885

Merged
merged 2 commits into from
Jun 5, 2023
Merged

Conversation

pinalj
Copy link
Contributor

@pinalj pinalj commented Jun 5, 2023

Made the encryption key variable and unique based on user email address to fix the vulneravility reported.

Made the encryption key variable and unique based on user email address to fix the vulneravility reported.
@pinalj pinalj added this to the 5.15.0 milestone Jun 5, 2023
@pinalj pinalj requested a review from handelce June 5, 2023 05:11
@pinalj pinalj self-assigned this Jun 5, 2023
Copy link
Contributor

@handelce handelce left a comment

Choose a reason for hiding this comment

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

@pinalj It all looks good. Big thumbs up for this one.

@pinalj pinalj merged commit 2084f77 into master Jun 5, 2023
1 check passed
@Ayantaker
Copy link

Ayantaker commented Jun 10, 2023

I was trying to look into CVE-2023-2986 (I created a POC script here : https://github.com/Ayantaker/CVE-2023-2986) which I guess this fix is for.

I tried out the latest plugin version and the vulnerable version 5.14.2 in a wordpress docker instance and the encryption key is set to be empty for both the instance, which makes it still vulnerable (Not sure if I configured something wrong or not )?

@ashokrane
Copy link
Contributor

@Ayantaker Thank you for testing this out. We are floating this back to our team to look into & will get back to you.

@Ayantaker
Copy link

Hi, any update on this ? I see a new version has released.

@pinalj
Copy link
Contributor Author

pinalj commented Jun 14, 2023

Hi @Ayantaker ,

We had allowed backward compatibility for older links to ensure cart recovery is not affected. In yesterday's release the same has been now removed to ensure the patch is completely secure.

@Ayantaker
Copy link

Ayantaker commented Jun 14, 2023

Hi @pinalj , Not sure I fully grasped that, but have a few questions :

  • Since the encryption is empty even in 5.15.0, that is still a vulnerability right ? Since it's in a different version than CVE-2023-2986, I think it makes sense to have a separate CVE for it registered. What do you think ?
  • I tried out the 5.15.1 version as well, with my POC script it still gets exploited.

@handelce
Copy link
Contributor

@Ayantaker

We have tested the vulnerability on 5.15.1 with your POC script and it looks good. For your reference, we have set up two staging sites. Please find their details below:

Unpatched: 5.14.2
URL : http://wcal-unpatched.instawp.xyz/
Username : yujasateni7589
Password : Mws09oCTLG2ezcKEpFA4

Patched: 5.15.1
URL : http://wcal-5-15-1.instawp.xyz/
Username : pefocaluji9285
Password : OTMIH6v0BwE459mKnoLa

Carts have been abandoned on both sites. The unpatched version reveals the vulnerability: php poc.php http://wcal-unpatched.instawp.xyz 80 10 while the patched version does not: php poc.php http://wcal-5-15-1.instawp.xyz 80 10

Please let me know if you have any questions.

@Ayantaker
Copy link

Hi @handelce , I have actually updated my POC script, not sure if you ran it against the updated one or not, but as you can see it's still able to exploit the version 5.15.1 instance of yours

php poc.php http://wcal-5-15-1.instawp.xyz/ 80 5
[*] Enumerating cart ID : 1
[*] Enumerating cart ID : 2
[+] Authentication Bypass URL for user 'pefocaluji9285' : http://wcal-5-15-1.instawp.xyz/:80/?wcal_action=checkout_link&user_email=test&validate=HAGYYG2fiWSJpGhKcwuX4ncOQFMwqB75JF9FTTxvmIREvHWhZoxhY7Yciv/2dJnOZSAVby42Ev1oig==
[*] Enumerating cart ID : 3
[*] Enumerating cart ID : 4
[*] Enumerating cart ID : 5

@handelce
Copy link
Contributor

@Ayantaker Got that. I'll check the updated script and let you know.

@handelce
Copy link
Contributor

@Ayantaker Thank you for your dedicated effort in seeing that this vulnerability is properly handled. We've taken a look at your POC script and based on the exploit, we've patched the plugin. We'll be releasing 5.15.2 today and we'd like for you to confirm from your end that things are working fine. Thanking you once more!

@Ayantaker
Copy link

Hey @handelce , if you have tested with the updated POC, should be good to go. Thanks for fixing it!

I would recommend filing for a new CVE ID for versions 5.15.0 and 5.15.1 since the previous CVE doesn't cover these versions or let me know I can request for one as well.

@handelce
Copy link
Contributor

@Ayantaker Yes, please go ahead. Thank you once more!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants