Closed
Bug 269125
Opened 20 years ago
Closed 19 years ago
img.onerror should fire for non-existent files
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
People
(Reporter: ecfbugzilla, Assigned: ecfbugzilla)
References
(Depends on 2 open bugs)
Details
(Keywords: testcase)
Attachments
(2 files, 1 obsolete file)
(deleted),
text/html
|
Details | |
(deleted),
patch
|
ecfbugzilla
:
review+
ecfbugzilla
:
superreview+
|
Details | Diff | Splinter Review |
Currently, the Image object allows probing of local files - for existent files
the error event is fired, for non-existent it isn't. Fortunately, the latest
Mozilla nightlies and Firefox 1.0 don't allow loading local images from remote
sites anymore, in Mozilla 1.7.3 this problem is still abusable remotely though.
This could be used in combination with some other exploit for example to
determine the location of the Windows directory by probing c:\windows\explorer.exe.
Assignee | ||
Comment 1•20 years ago
|
||
This testcase will only work remotely in Mozilla 1.7.3, for Firefox 1.0 or
Mozilla 1.8a you will have to download it and open from file:///. Enter the
path to any file into the text field e.g. c:\windows\explorer.exe and press
"Check file" - an alert will appear and tell you whether this file exists in
your system.
Assignee | ||
Comment 2•20 years ago
|
||
This makes nsImageLoadingContent fire error events in some more cases,
especially if nsContentUtils::LoadImage() fails. This is the case if the
channel couldn't be created, for example for a non-existent local file. There
is still no error event if an image loaded via HTTP is missing but this is
another issue and probably has to be fixed in imgRequest.
Assignee | ||
Comment 3•20 years ago
|
||
Comment on attachment 165570 [details] [diff] [review]
Patch
I noticed that local images loaded from remote pages is bug 69070, maybe it's
best for jst to review then...
Attachment #165570 -
Flags: review?(jst)
Comment 4•20 years ago
|
||
> There is still no error event if an image loaded via HTTP is missing
Wouldn't that parse the 404 page as an image? In which case, imagelib should be
reporting some sort of error status to us in OnStopDecode, right?
Assignee | ||
Comment 5•20 years ago
|
||
You are right, it does. Ignore this comment...
Assignee | ||
Comment 6•20 years ago
|
||
*** Bug 252961 has been marked as a duplicate of this bug. ***
Comment 7•20 years ago
|
||
with Bug 252961 public, does this bug need to stay marked security sensitive?
Updated•20 years ago
|
Group: security
Comment 8•19 years ago
|
||
Comment on attachment 165570 [details] [diff] [review]
Patch
r=jst
Attachment #165570 -
Flags: superreview?(bzbarsky)
Attachment #165570 -
Flags: review?(jst)
Attachment #165570 -
Flags: review+
Comment 9•19 years ago
|
||
Comment on attachment 165570 [details] [diff] [review]
Patch
sr=bzbarsky (though I'm not sure this is the right thing to do if
!mLoadingEnabled... but it can't be too bad).
Attachment #165570 -
Flags: superreview?(bzbarsky) → superreview+
Assignee | ||
Comment 10•19 years ago
|
||
Bitrot removed
Attachment #165570 -
Attachment is obsolete: true
Attachment #192970 -
Flags: superreview+
Attachment #192970 -
Flags: review+
Comment 11•19 years ago
|
||
Thanks for the updated patch, fix is now checked in.
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Would anyone be willing to port this to the 1.8.1 branch if given approval to land? This bug is making it hard to detect when a load failed. See bug 317383
Comment 13•19 years ago
|
||
*** Bug 328687 has been marked as a duplicate of this bug. ***
Assignee | ||
Updated•18 years ago
|
Assignee: general → trev
Comment 14•16 years ago
|
||
I just figured that this bug has been fixed for Firefox 3 but not in Firefox 2.0.0.16.
By the way, the testcase is misleading as it does not test what the title claims: As I take it, this bug is about whether the img.onerror handler fires on local files or not. The testcase sets img.onload = img.onerror so it can not test this. Check Bug 252961 for a valid testcase.
Comment 15•16 years ago
|
||
Said valid testcase should be checked in as a regression test...
Flags: in-testsuite?
You need to log in
before you can comment on or make changes to this bug.
Description
•