HTML directory indexer doesn't html-escape url

VERIFIED FIXED in mozilla1.0.1

Status

()

Core
Security
P1
critical
VERIFIED FIXED
16 years ago
16 years ago

People

(Reporter: bbaetz, Assigned: bbaetz)

Tracking

Trunk
mozilla1.0.1
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [FIXED ON TRUNK][FIXED ON BRANCH], URL)

Attachments

(1 attachment)

1.86 KB, patch
dougt
: review+
Judson Valeski
: superreview+
Judson Valeski
: approval+
Details | Diff | Splinter Review
(Assignee)

Description

16 years ago
As reported by ptrs-ejy@bp.iij4u.or.jp to the security group, the uri (which is
added to the page) isn't html escaped:

<quote>
+ Exploit code:
~~~~~~~~~~~~~~~~~
<a href="ftp://'FTPserver' or
'FTP+HTTPserver'/#%3C%2ftitle%3E%3Cscript%3Ealert(%22exploit%22);%3C%2fscript%3E">Exploit</a>
</quote>

(You need to add a valid ftp server in there)

Patch coming
(Assignee)

Comment 1

16 years ago
Created attachment 89011 [details] [diff] [review]
patch

This rearranges code a bit so that we don't leak while using the raw pointers
the escaping functions require (Also fixes an existing leak of |spec|, too)
(Assignee)

Updated

16 years ago
Status: NEW → ASSIGNED
Keywords: patch, review
Target Milestone: --- → mozilla1.0.1

Comment 2

16 years ago
Comment on attachment 89011 [details] [diff] [review]
patch

r-dougt
Attachment #89011 - Flags: review+
(Assignee)

Comment 3

16 years ago
*** Bug 154029 has been marked as a duplicate of this bug. ***

Comment 4

16 years ago
Comment on attachment 89011 [details] [diff] [review]
patch

>Index: nsIndexedToHTML.cpp

>+    char* spec = nsCRT::strdup(titleUri.get());
>+    nsXPIDLString unEscapeSpec;
>+    rv = mTextToSubURI->UnEscapeAndConvert(encoding, spec,
>+                                           getter_Copies(unEscapeSpec));
>+    nsCRT::free(spec);

why strdup?  second param to UnEscapeAndConvert is |const char *|, so
what's wrong with |titleUri.get()|?

justify/fix and sr=darin
(Assignee)

Comment 5

16 years ago
darin: Historical reasons - that used to call nsUnescape.

Checked in with that change to the trunk; I'll mail drivers for the branch.
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Keywords: approval, mozilla1.0.1
Priority: -- → P1
Resolution: --- → FIXED
Whiteboard: [FIXED ON TRUNK]

Comment 6

16 years ago
Comment on attachment 89011 [details] [diff] [review]
patch

adding darin's sr.
Attachment #89011 - Flags: superreview+
Attachment #89011 - Flags: approval+

Comment 7

16 years ago
please checkin to the 1.0.1 branch. once there, remove the "mozilla1.0.1+"
keyword and add the "fixed1.0.1" keyword.
Keywords: mozilla1.0.1 → mozilla1.0.1+
(Assignee)

Comment 8

16 years ago
Fixed on branch, too (after my checkin last night was backed out because the
tree was closed. Oops)
Keywords: mozilla1.0.1+ → fixed1.0.1
Whiteboard: [FIXED ON TRUNK] → [FIXED ON TRUNK][FIXED ON BRANCH]

Comment 9

16 years ago
Verified on 2002-06-27-trunk and 2002-06-27-branch build on Win2K.

Above URL displays the direcrtory contents.
Status: RESOLVED → VERIFIED
Keywords: fixed1.0.1 → verified1.0.1

Updated

16 years ago
Component: Networking: FTP → Security: General
Group: security?
You need to log in before you can comment on or make changes to this bug.