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

RESOLVED FIXED

Status

()

Core
Networking: HTTP
--
critical
RESOLVED FIXED
9 years ago
6 years ago

People

(Reporter: Todd Agulnick, Assigned: timeless)

Tracking

({crash})

unspecified
crash
Points:
---
Bug Flags:
wanted1.9.0.x ?
wanted1.8.1.x +
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

(crash signature)

Attachments

(1 attachment)

Comment hidden (empty)
(Reporter)

Comment 1

9 years ago
(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);
Summary: Setting "Authorization" request header to value without a space crashes → Setting "Authorization" request header to value without a space results in crash
(Assignee)

Comment 2

9 years ago
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
Severity: normal → critical
Keywords: crash
OS: Linux → All
Hardware: Other → All
Summary: Setting "Authorization" request header to value without a space results in crash → Setting "Authorization" request header to value without a space results in crash [@ libc-2.6.so@0x6fbcc]
(Assignee)

Comment 3

9 years ago
Created attachment 343363 [details] [diff] [review]
recycle code
Assignee: nobody → timeless
Status: NEW → ASSIGNED
Attachment #343363 - Flags: review?(cbiesinger)
Comment on attachment 343363 [details] [diff] [review]
recycle code

This seems like a good patch for the branches as well.
Attachment #343363 - Flags: superreview+
Attachment #343363 - Flags: review?(cbiesinger)
Attachment #343363 - Flags: review+
Attachment #343363 - Flags: approval1.9.0.4?
Attachment #343363 - Flags: approval1.8.1.18?
Attachment #343363 - Flags: approval1.8.0.15?
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.
Attachment #343363 - Flags: approval1.9.0.5?
Attachment #343363 - Flags: approval1.9.0.4?
Attachment #343363 - Flags: approval1.8.1.19?
Attachment #343363 - Flags: approval1.8.1.18?
Attachment #343363 - Flags: approval1.9.0.5?
Attachment #343363 - Flags: approval1.8.1.19?
Attachment #343363 - Flags: approval1.8.0.15?
Comment on attachment 343363 [details] [diff] [review]
recycle code

Please re-request approval after this has landed on trunk.
Flags: wanted1.9.0.x+
Flags: wanted1.8.1.x+
(Assignee)

Comment 7

9 years ago
fixed in changeset 1ba5609a213a
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
(Assignee)

Updated

9 years ago
Attachment #343363 - Flags: approval1.9.0.5?
Attachment #343363 - Flags: approval1.8.1.19?
Attachment #343363 - Flags: approval1.8.0.15?
Flags: in-testsuite?
Attachment #343363 - Flags: approval1.9.0.5?
Attachment #343363 - Flags: approval1.8.1.19?
Attachment #343363 - Flags: approval1.8.0.15?
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.
Flags: wanted1.9.0.x+ → wanted1.9.0.x?
Crash Signature: [@ libc-2.6.so@0x6fbcc]
You need to log in before you can comment on or make changes to this bug.