"FireFTP", vulnerability of injection script to chrome

RESOLVED INVALID

Status

P2
major
RESOLVED INVALID
8 years ago
3 years ago

People

(Reporter: alice0775, Assigned: jorgev)

Tracking

unspecified
5.12
x86
Windows 7

Firefox Tracking Flags

(Not tracked)

Details

(URL)

(Reporter)

Description

8 years ago
Japanese blog reported.
http://d.hatena.ne.jp/teramako/20100621/p1 (in japanese)

Install add on "FireFTP" ( hhttps://addons.mozilla.org/en-US/firefox/addon/684/ ),
the FTP "welcome message" can inject script to chrome.

gFxp.welcomeMessage is not sanitized.

Actual
the FTP "welcome message" can inject script to chrome.

Expected 
gFxp.welcomeMessage should be sanitized.

It is difficult to prepare testcase. However, this vulnerability is as same as WizzRSS'vulnerability.
Assignee: nobody → jorge
(Assignee)

Updated

8 years ago
Severity: normal → major
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Priority: -- → P2
Target Milestone: --- → 5.12
(Assignee)

Comment 1

8 years ago
Adding author to CC list.

Comment 2

8 years ago
Just returned from an extended vacation - I will look into it as soon as I can.

Comment 3

8 years ago
Apologies for the delay in responding - I've been away for 2 months building a house in the middle of nowhere with internet connection!  Finally got my computer up and running again...

Anyway, unless I'm mistaken this seems to be a duplicate of bug 476851 which was already resolved last year.

And then furthermore, the only other place welcomeMessage is used is in welcome.xul which inserts the message into a textbox, not an HTML capable widget.
see: http://www.mozdev.org/source/browse/fireftp/src/content/welcome.xul?rev=1.8

Please check my logic and review bug 476851.
(Assignee)

Comment 4

8 years ago
Adding Wladimir, who analyzed the previous fixes. Could you please have a look?

According to the blog (as far as I understand), they didn't verify the exploit themselves, so it could be a false alarm.

Comment 5

8 years ago
Yes, seems to be a false alarm to me. Bug 476851 was about a different issue, handling of the welcome message was safe even then. What I can see:

* gFtp.welcomeMessage is being set by connection/controlSocket.js. No sanitizing performed but it isn't necessary either.

* gFtp.welcomeMessage is used in a few locations, always as a parameter to displayWelcomeMessage(). This function opens chrome://fireftp/content/welcome.xul and passes the message as a window argument.

* Further processing in welcome.xul is very simple:

> $('message').value = window.arguments[0];

Here $('message') is a multi-line textbox. So it will interpret the message as text, absolutely no issue here.

I also checked for uses of innerHTML, just in case. Function appendLog() is still the only problematic case. Sanitizing is performed only if the parameter "untrusted" is set. There are a few direct calls to appendLog() without that parameter but these are safe. It's more problematic when appendLog() is called indirectly through error() function - quite a few of these calls don't have the "untrusted" parameter set despite dealing with data from uncertain sources. Typical example:

> error(gStrbundle.getFormattedString("failedDir", [remotePath]));

I'm not sure how much the remote server can manipulate the contents of remotePath variable. Nor am I really inclined to investigate this in detail because we are talking about unnecessary risk here - failedDir string doesn't even contain any HTML code. The approach was apparently to set "untrusted" parameter only for strings that are definitely not safe. I would recommend doing it exactly the other way around: have a "trusted" parameter in appendLog() that needs to be set if a string is definitely safe and contains HTML code. And then only set it in the few cases where it is absolutely necessary.

The same tendency continues with debugging functions (which shouldn't affect most users because fireftp.debugmode preference needs to be changed). detailedError() function never passes "untrusted" parameter - but seems to be unused anyway. debug() function however is used a lot and usually without "untrusted" parameter, often with potentially unsafe parameters. From what I can tell, none of the debug calls are intended to write HTML code so this risk is again unnecessary.
(Assignee)

Comment 6

8 years ago
Resolving this as Invalid.

Thank you all!
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → INVALID

Comment 7

8 years ago
Thanks for the analysis and confirmation, Wladimir.  I will take your suggestion to make trusted the default.
(Assignee)

Updated

3 years ago
Group: client-services-security
You need to log in before you can comment on or make changes to this bug.