Dhiraj Mishra Reporter
	Description • 7 years ago

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:57.0) Gecko/20100101 Firefox/57.0
Build ID: 20171206182557

Steps to reproduce:

Hi Team, 

During sign in into https://bugzilla.mozilla.org via GitHub, the request shares a github_secret key which is of 15 characters apart from that there is no rating limit for the same, I was able to send 30+ dummy packet to github_secret parameter while 40th request had mu actual secret key and the status change 302 for that.
However this could give an edge to attacker of the actual secret key.

Attaching the poc for the same. 


Actual results:

Has there is no rating limit and  github_secret key length is also weak so the attack surface over here becomes high.


Expected results:

As expected Bugzilla.mozilla.org should have rating limit set for it or 128 bit long github_secret key.

As per the OWASP guideline Session identifiers should be at least 128 bits long to prevent brute-force session guessing attacks. OWASP provided below example

“With a 64 bit session identifier, assume 32 bits of entropy. For a large web site, assume that the attacker can try 1,000 guesses per second and that there are 10,000 valid session identifiers at any given moment. Given these assumptions, the expected time for an attacker to successfully guess a valid session identifier is less than 4 minutes.

Now assume a 128 bit session identifier that provides 64 bits of entropy. With a very large web site, an attacker might try 10,000 guesses per second with 100,000 valid session identifiers available to be guessed. Given these assumptions, the expected time for an attacker to successfully guess a valid session identifier is greater than 292 years.”

	Dylan Hardison [:dylan] (he/him) Assignee
	Comment 1 • 7 years ago

I think we can dump up the length, but you would also need to set a cookie with the same value for this to be a problem, IIRC. I'll ask simon to look into this and rate it.

	Dylan Hardison [:dylan] (he/him) Assignee
	Comment 2 • 7 years ago

The token was added for bug 1196743

	Simon Bennetts [:psiinon]
	Comment 3 • 7 years ago

Agree with the recommendations to increase the github_secret key length to at least 128 and to apply rate limiting.

Thanks for reporting Dhiraj - I've flagged this for a possible bug bounty - bugzilla is one of the eligible sites.

	Simon Bennetts [:psiinon]
	Comment 4 • 7 years ago

Its worth noting that the OWASP guidelines quoted above are for brute forcing session identifiers, where there is a relatively large pool active, any of which can be matched.
In this case there should be only one valid secret key for a given user, which reduces the target space.

	Dylan Hardison [:dylan] (he/him) Assignee
	Comment 6 • 7 years ago

This patch:

1. adds functionality to add new categories to the rate limit rule (an oversight from its implementation
2. extends the logging format to include the rate limit category
3. increases the github_secret to 256 chars, which is I think is 2.04586913e149 possible values.
4. adds rate limiting on the github_secret value
5. adds rate limiting on the github_state checking too.

To hit the rate limiting would require setting a cookie on bugzilla.mozilla.org or mozilla.org,
and as such I imagine watching for the rate limit events would be useful/interesting.

	David Lawrence [:dkl]
	Comment 7 • 6 years ago

Comment on attachment 8938120 [details] [diff] [review]
1426409_1.patch

Review of attachment 8938120 [details] [diff] [review]:
-----------------------------------------------------------------

r=dkl

	Dylan Hardison [:dylan] (he/him) Assignee
	Comment 8 • 6 years ago

This will go out tomorrow morning.

	Simon Bennetts [:psiinon]
	Comment 9 • 6 years ago

Thanks Dylan!

	Dhiraj Mishra Reporter
	Comment 10 • 6 years ago

Thank you dylan, i was OOO just crossed checked the issue this seems to be fix !

	April King [:April]
	Comment 11 • 6 years ago

62 possible characters, 15 character length, 10000 active users, and 10000 guesses/second would require approximately 9.4M days to find a single collision on average. Note that 10000 guesses/second is far beyond what our current infrastructure can support. If it's just upper and lower case letters, that's still 2.5M/days by my math.

	Dhiraj Mishra Reporter
	Comment 12 • 6 years ago

(In reply to April King [:April] from comment #11)
> 62 possible characters, 15 character length, 10000 active users, and 10000
> guesses/second would require approximately 9.4M days to find a single
> collision on average. Note that 10000 guesses/second is far beyond what our
> current infrastructure can support. If it's just upper and lower case
> letters, that's still 2.5M/days by my math.

Argghh I see, No bounty though :( 
However, i request if someone could take time to vouch my profile will be happy. https://mozillians.org/en-US/u/Dhiraj_Mishra/

	April King [:April]
	Comment 13 • 6 years ago

To clarify for the OP, "15 characters" is not same as "15 bits". With 15 characters and a keyspace of 62 possible alphanumerics, a random github_secret key would have something along the lines of 88 bits of entropy.

While that is certainly less than 128 bits of entropy, it's pretty infeasible to brute force remotely, especially given Bugzilla's performance constraints. 64 bits may take 4 minutes to brute force at 1k requests/sec (far beyond what BMO can handle), but each bit of entropy doubles the amount of time necessary and so 88 bits is much higher than 64.

	Dylan Hardison [:dylan] (he/him) Assignee
	Comment 14 • 6 years ago

Also notice the mitigation here -- the github secret -- only protects against unauthorized redirections /to bugzilla itself/.
I'm not sure if it was even moderate.

Someone looking at this from 10,000 miles up might assume that the github secret has something to do with github oauth flow or perhaps the state parameter used there-in, but it doesn't. :-)