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

Release 2.15.2 #7744

Merged
merged 28 commits into from Aug 1, 2019

Conversation

@kuzmany
Copy link
Contributor

commented Jul 31, 2019

Please be sure you are submitting this against the staging branch.

Q A
Bug fix?
New feature?
Automated tests included?
Related user documentation PR URL
Related developer documentation PR URL
Issues addressed (#s or URLs)
BC breaks?
Deprecations?

Description:

Steps to reproduce the bug:

Steps to test this PR:

  1. Load up this PR

List deprecations along with the new alternative:

List backwards compatibility breaks:

heathdutton and others added some commits Dec 10, 2018

Merge pull request #50 from mautic-inc/escape-validation-errors
Escape validation error messages to avoid HTML tag rendering in the UI
Merge pull request #47 from mautic-inc/security.open-redirect
Prevent open redirect vulnerability
@escopecz
Copy link
Member

left a comment

Eh, Travis has some problems.

@escopecz

This comment has been minimized.

Copy link
Member

commented Jul 31, 2019

Aha, this is not a issue of incoming changes but on master. We missed it in #7582

@escopecz

This comment has been minimized.

Copy link
Member

commented Jul 31, 2019

@kuzmany I found it. @heathdutton prepared a fix for Travis and it was merged to staging:

#7611

That's why staging works and master is falling. Add that PR to 2.15.2 release (this PR) and everything should be OK.

@kuzmany

This comment has been minimized.

Copy link
Contributor Author

commented Jul 31, 2019

Now it fails due your unserialize second parameter (already discussed)
https://travis-ci.org/mautic/mautic/jobs/566041387

Release it with travis error?

@Gregy

This comment has been minimized.

Copy link
Contributor

commented Aug 1, 2019

I think we need to. We should also publish a release notes that strongly discourage using mautic with php 5.x because the vulnerability is NOT fixed in php 5.x. Meaning anyone can easily take over any mautic installation running on old php (even when they use the latest mautic)

Alternatively the vulnerability would need to be fixed for php5x too but that would be pretty difficult to do correctly.

@escopecz

This comment has been minimized.

Copy link
Member

commented Aug 1, 2019

Let me play with it a little more. Maybe I'll be able to create a helper to encapsulate all unserializing and check the PHP version in it.

I don't want to have PHP 5.6 failing on master just because of this.

@escopecz

This comment has been minimized.

Copy link
Member

commented Aug 1, 2019

Looks like we are green and ready. @kuzmany please make a review of the last commits before merging. I checked the code 3 times but a second pair of eyes may find something I missed.

app/bundles/CoreBundle/Helper/Serializer.php Outdated Show resolved Hide resolved
@Gregy

Gregy approved these changes Aug 1, 2019

@Gregy

This comment has been minimized.

Copy link
Contributor

commented Aug 1, 2019

Thank you

@kuzmany kuzmany merged commit de5fc98 into master Aug 1, 2019

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.