Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BS2] Fix for XSS issue with data-target attributes in BS2 JS #22844

Merged
merged 5 commits into from Mar 8, 2019

Conversation

Projects
None yet
9 participants
@zero-24
Copy link
Contributor

zero-24 commented Oct 28, 2018

Summary of Changes

This patch by @SniperSister fixes an publicly known XSS Problem in the BS2 JS reported to the JSST by @C-Lodder. As this issue is publicly known and can also impact 3rd partys the JSST decided to patch it in the public tracker. This also allows an wider group of people to test this patch.

Testing Instructions

Make sure the following bootstrap js components still work

  • Alert
  • Carousel
  • Collapse
  • Dropdown
  • Modal
  • Scrollspy
  • Tab

Try using the data-target and href methods and also try to verify that the mentioned components still work in the Joomla backend.

Expected result

BS JS Code still works

Actual result

The current JS Code has an know and publicly documented XSS Problem

Documentation Changes Required

none

cc @joomla/security @mbabker

@zero-24 zero-24 added this to the Joomla 3.9.1 milestone Oct 28, 2018

@mbabker mbabker removed this from the Joomla 3.9.1 milestone Nov 23, 2018

@zero-24

This comment has been minimized.

Copy link
Contributor Author

zero-24 commented Feb 18, 2019

We might can get some tests or some kind of Feedback here?

What does hold you off testing? Is it unclear what to test?
Whatvholds you up providing Feedback? Let me know we really want to get this fixed. cc @SniperSister

@alikon

This comment has been minimized.

Copy link
Contributor

alikon commented Feb 21, 2019

tested

  • alert
  • collapse
  • dropdown
  • modal
  • tab

and still works in the Joomla backend

unable to test carousel & scrollspy (don't know where these are used in the backend)
sorry i'm not a js guy 🤕

@HLeithner

This comment has been minimized.

Copy link
Member

HLeithner commented Feb 22, 2019

Any chance to get a look on this from @dgrammatiko ?

@dgrammatiko

This comment has been minimized.

Copy link
Contributor

dgrammatiko commented Feb 22, 2019

@HLeithner looks fine here

@HLeithner

This comment has been minimized.

Copy link
Member

HLeithner commented Feb 22, 2019

@PhocaCz could you please test this PR with some of your extensions that maybe use this feature set?

@PhocaCz

This comment has been minimized.

Copy link

PhocaCz commented Feb 22, 2019

@HLeithner

Is this related to frontend or backend? In frontend, I use BS3 for my extensions.

@HLeithner

This comment has been minimized.

Copy link
Member

HLeithner commented Feb 22, 2019

@PhocaCz its front and backend core BS2

@PhocaCz

This comment has been minimized.

Copy link

PhocaCz commented Feb 22, 2019

I have tested this item successfully on f3d9afb

Tested Joomla! 3.9.3 administration: Alert, Modal, Tab with 3pd extensions (Phoca Gallery, Phoca Download, Phoca Cart) and it seems like everything is OK


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/22844.

@zero-24 zero-24 added this to the Joomla 3.9.4 milestone Feb 23, 2019

@Razzo1987

This comment has been minimized.

Copy link

Razzo1987 commented Feb 23, 2019

I have tested this item successfully on 6980056

I have tested the Carousel creating a custom code with: https://getbootstrap.com/2.3.2/javascript.html#carousel


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/22844.

@joomla-cms-bot joomla-cms-bot removed this from the Joomla 3.9.4 milestone Feb 23, 2019

@Quy

This comment has been minimized.

Copy link
Contributor

Quy commented Feb 23, 2019

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/22844.

@joomla-cms-bot joomla-cms-bot added the RTC label Feb 23, 2019

@zero-24

This comment has been minimized.

Copy link
Contributor Author

zero-24 commented Feb 23, 2019

Thanks for your tests 👍 cc to be merged @HLeithner

@zero-24 zero-24 added this to the Joomla 3.9.4 milestone Feb 23, 2019

@HLeithner

This comment has been minimized.

Copy link
Member

HLeithner commented Feb 23, 2019

I would like to see the JED tested with it

@mbabker

This comment has been minimized.

Copy link
Member

mbabker commented Feb 23, 2019

I would like to see the JED tested with it

This is not a practical request. Extension code is not stored in the JED or in a resource available within the JED, and it is not within the JED's role in relation to core development to audit or test extensions against any core change.

@HLeithner

This comment has been minimized.

Copy link
Member

HLeithner commented Feb 23, 2019

My request is to test the jed, not extensions from jed.

And don't worry I will do it my self.

@HLeithner HLeithner merged commit 8cc9186 into joomla:staging Mar 8, 2019

0 of 3 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/drone/pr this build is pending
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
@HLeithner

This comment has been minimized.

Copy link
Member

HLeithner commented Mar 8, 2019

thx all for there work.

@joomla-cms-bot joomla-cms-bot removed the RTC label Mar 8, 2019

@zero-24 zero-24 deleted the zero-24:bs2_patch branch Mar 11, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.