Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 1063853 - XSS vulnerability in changePasswordMsg
Summary: XSS vulnerability in changePasswordMsg
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: oVirt
Classification: Retired
Component: ovirt-engine-userportal
Version: 3.4
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
: 3.4.0
Assignee: Yair Zaslavsky
QA Contact: Petr Beňas
URL:
Whiteboard: infra
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2014-02-11 14:31 UTC by Tomas Jelinek
Modified: 2015-03-05 00:19 UTC (History)
13 users (show)

Fixed In Version: ovirt-3.4.0-beta3
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2014-03-31 12:27:00 UTC
oVirt Team: ---


Attachments (Terms of Use)
xss (deleted)
2014-02-11 14:31 UTC, Tomas Jelinek
no flags Details


Links
System ID Priority Status Summary Last Updated
oVirt gerrit 24006 None None None Never
oVirt gerrit 24272 None None None Never
oVirt gerrit 24282 None None None Never
oVirt gerrit 24283 None None None Never

Description Tomas Jelinek 2014-02-11 14:31:19 UTC
Created attachment 861796 [details]
xss

The changePasswordMsg does not escape html tags. For example the following message results in the screenshot attached.

http://"></a> <img src="http://www.empireonline.com/images/uploaded/chuck-norris-uzis.jpg" /> <a href=""

It has the first http:// because the FE is trying to make <a href... out of the first http:// occurrence.

Comment 1 Itamar Heim 2014-02-11 20:31:51 UTC
its XSS if you don't trust the ovirt admin, or possible to inject by another site somehow?
becuse the ovirt admin can send whatever they want to the clients...

Comment 2 Tomas Jelinek 2014-02-12 07:05:09 UTC
(In reply to Itamar Heim from comment #1)
> its XSS if you don't trust the ovirt admin, or possible to inject by another
> site somehow?

only if you are able to manipulate the vdc_options

> becuse the ovirt admin can send whatever they want to the clients...

well, it is not a serious security vulnerability and I can hardly imagine an attack done using it. 

But I still think that we should handle it properly because this kind of mistakes do not really fill the user with trust...

Comment 3 Itamar Heim 2014-02-12 08:18:57 UTC
an admin can simply change the entire website for anything they want...
also, what if the admin does want to have some javascript in that message, etc?

Comment 4 Tomas Jelinek 2014-02-12 08:45:39 UTC
(In reply to Itamar Heim from comment #3)
> an admin can simply change the entire website for anything they want...
> also, what if the admin does want to have some javascript in that message,
> etc?

Is this a meant use case? If yes, we should probably document it at least in the engine-manage-domains help otherwise it looks like a bug. But in that case we should support it a bit better because it is not that simple to make a javascript executable nor to reference something external (like an image).

Comment 5 Petr Matousek 2014-02-12 15:06:51 UTC
(In reply to Itamar Heim from comment #3)
> an admin can simply change the entire website for anything they want...
> also, what if the admin does want to have some javascript in that message,
> etc?

I agree with Itamar. It looks like you need to be highly privileged to perform the attack and when you are, you have other means to do the same (or even worse things).

Comment 6 Tomas Jelinek 2014-02-12 15:36:38 UTC
(In reply to Petr Matousek from comment #5)
> (In reply to Itamar Heim from comment #3)
> > an admin can simply change the entire website for anything they want...
> > also, what if the admin does want to have some javascript in that message,
> > etc?
> 
> I agree with Itamar. It looks like you need to be highly privileged to
> perform the attack and when you are, you have other means to do the same (or
> even worse things).

So, what the outcome from this discussion is?
- is it a bug not worth fixing because the consequences are too small?
- is it a bug worth fixing because the fix is a one liner and it is simply not correct to trust that string?
- is it a feature and so we will document it?
- will we let it as an undocumented feature?

Comment 7 Petr Matousek 2014-02-12 15:44:19 UTC
(In reply to Tomas Jelinek from comment #6)
> So, what the outcome from this discussion is?

I solely commented on the potential security impact this issue might have.

Comment 8 Yair Zaslavsky 2014-02-12 21:04:41 UTC
Alexander, IMHO change-id I6c78c9e6d6dc9417e22c71dab66ae4507ea5c191  [1] should handle these issues , am I correct?


[1] 
http://gerrit.ovirt.org/#/c/24282/
http://gerrit.ovirt.org/#/c/24006/

Comment 9 Alexander Wels 2014-02-12 21:19:30 UTC
Yair,

Yes I created those patches specifically to stop any potential XSS attacks. The application should not trust anything coming from an external resource (in this case the database is an external resource). If the XSS exists after those patches are applied, then that is a bug that should be fixed. I am going to test right now if master still shows this behaviour.

Comment 10 Alexander Wels 2014-02-12 21:29:51 UTC
Yair,

I just confirmed that those patches do solve the XSS vulnerability and they have been backported to 3.4 already. So we can move this to modified after I add the links to the patches to this bug?

Comment 11 Yair Zaslavsky 2014-02-12 21:40:42 UTC
(In reply to Alexander Wels from comment #10)
> Yair,
> 
> I just confirmed that those patches do solve the XSS vulnerability and they
> have been backported to 3.4 already. So we can move this to modified after I
> add the links to the patches to this bug?

Yes please

Comment 12 Itamar Heim 2014-02-16 08:23:27 UTC
Setting target release to current version for consideration and review. please
do not push non-RFE bugs to an undefined target release to make sure bugs are
reviewed for relevancy, fix, closure, etc.

Comment 13 Petr Beňas 2014-02-21 14:18:54 UTC
Verified in ovirt-engine-userportal-3.4.0-0.11.beta3.el6.noarch.
Image is not dispalyed, tags gets escaped as:

Cannot Login. User Password has expired. Use the following URL to change the password: http://%22%3E%3C/a%3E <img src="http://www.empireonline.com/images/uploaded/chuck-norris-uzis.jpg" /> <a href=""

Comment 14 Sandro Bonazzola 2014-03-31 12:27:00 UTC
this is an automated message: moving to Closed CURRENT RELEASE since oVirt 3.4.0 has been released


Note You need to log in before you can comment on or make changes to this bug.