460115 - Setting "Authorization" request header to value without a space results in crash [@ libc-2.6.so@0x6fbcc]

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect)

Comment 1

[Todd Agulnick]

(apologies about the premature submission)

If a client sets the "Authorization" field to a value that doesn't contain a space, the result is a crash as, for example, here:

    http://crash-stats.mozilla.com/report/index/ddf890a3-9a61-11dd-a2b3-001cc45a2ce4

That appears to be because Necko expects space between the auth-type (e.g., "Basic" or "Digest") and the credentials. That space does indeed appear to be required by the spec (and is certainly present by convention), but protecting against its absence still seems like a good idea.

THe problem appears to be here: http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/netwerk/protocol/http/src/nsHttpChannel.cpp&rev=1.333&mark=2152#2152

    nsCAutoString buf(Substring(val, strchr(val, ' ')));

If the value for the authorization header (val) doesn't contain a space, bad things ensue. I gather the desire is to strip off the auth type (why? to save memory?) but a safer approach might be:

    const char *space = strchr(val, ' ');
    nsCAutoString buf(space ? Substring(val, space) : val);

Comment 2

[timeless]

Signature	libc-2.6.so@0x6fbcc
UUID	ddf890a3-9a61-11dd-a2b3-001cc45a2ce4
Time	2008-10-14 19:34:48-07
Uptime	63
Product	Firefox
Version	3.0.1
Build ID	2008070206
OS	Linux
OS Version	0.0.0 Linux 2.6.23.17-88.fc7 #1 SMP Thu May 15 00:02:29 EDT 2008 x86_64 GNU/Linux
CPU	x86
CPU Info	GenuineIntel family 10 model 15 stepping 6
Crash Reason	SIGSEGV
Crash Address	0x23dbcc
Comments	
Crashing Thread
Frame 	Module 	Signature 	Source
0 	libc-2.6.so 	libc-2.6.so@0x6fbcc 	
1 	libxul.so 	nsACString_internal::Assign 	mozilla/xpcom/string/src/nsSubstring.cpp:406
2 	libxul.so 	nsCAutoString::nsCAutoString 	nsTString.h:530
3 	libxul.so 	nsHttpChannel::StoreAuthorizationMetaData 	mozilla/netwerk/protocol/http/src/nsHttpChannel.cpp:2152

Comment 3

[timeless]

Attachment: recycle code

Comment 4

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 343363 [details] [diff] [review]
recycle code

This seems like a good patch for the branches as well.

Comment 5

[Samuel Sidler (old account; do not CC)]

Comment on attachment 343363 [details] [diff] [review]
recycle code

This needs checkin on trunk and baking, but we'll consider it for the next branch release.

Comment 6

[Daniel Veditz [:dveditz]]

Comment on attachment 343363 [details] [diff] [review]
recycle code

Please re-request approval after this has landed on trunk.

Comment 7

[timeless]

fixed in changeset 1ba5609a213a

Comment 8

[Samuel Sidler (old account; do not CC)]

Comment on attachment 343363 [details] [diff] [review]
recycle code

This needs a testcase (automated for 1.9.0) before we'll take it on the branches.