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: unterminated img src causes long execution #257

Closed
wheresrhys opened this issue Jan 8, 2019 · 5 comments
Closed

Vulnerability: unterminated img src causes long execution #257

wheresrhys opened this issue Jan 8, 2019 · 5 comments

Comments

@wheresrhys
Copy link

The following test runner demonstrates the problem

const test = async (zeroes) => {
	const start = Date.now();
	const characterCount = Number('1' + [...Array(Number(zeroes))].map(() => '0').join(''))
	autolinker.link(`<img src="${[...Array(characterCount)].join('a')}`)
	console.log(`src of length ${characterCount} took ${Date.now() - start} ms`)
}


const testRunner = async degrees=> {

	const zeroes = [...Array(degrees)].map((_, i) => i);
	for (i in zeroes) {
		await test(i)
	}

}

testRunner(7)
@gregjacobs
Copy link
Owner

Hey @wheresrhys, thanks for this. I'll check it out!

@gregjacobs
Copy link
Owner

gregjacobs commented Jan 23, 2019

Figured it out: The regular expression which processed email addresses must have been doing a lot of backtracking on your input string. I replaced both the regexp-based html parser (#259) and email matcher (#260) with a state machine parser that runs in linear time. New output of your test driver with the changes:

src of length 1 took 0 ms
src of length 10 took 32 ms
src of length 100 took 2 ms
src of length 1000 took 3 ms
src of length 10000 took 11 ms
src of length 100000 took 98 ms
src of length 1000000 took 784 ms

Will be released in 3.0

@gregjacobs
Copy link
Owner

This is now up in 3.0. Let me know if you come across any other issues, and thanks for reporting!

@wheresrhys
Copy link
Author

That sounds like an epic rewrite. Thanks a lot 🥂

@gregjacobs
Copy link
Owner

It definitely was an epic rewrite! But a long time coming, and definitely needed :) Glad to help!

dereckson added a commit to nasqueron/servers-homepages that referenced this issue Dec 16, 2019
Summary: Fixes gregjacobs/Autolinker.js#257

Test Plan: `./build.js windriver`

Reviewers: dereckson

Reviewed By: dereckson

Differential Revision: https://devcentral.nasqueron.org/D2119
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

No branches or pull requests

2 participants