mockup icon indicating copy to clipboard operation
mockup copied to clipboard

JS Libraries with XSS vulnerabilities

Open frapell opened this issue 6 years ago • 14 comments

A customer has brought to our attention a Security Report informing that there are some known vulnerabilities with Javascript libraries, in particular with jQuery 1.11.3 and Bootstrap 3.3.6, which are being used in Mockup.

Information about them can be seen at:

  • https://bugs.jquery.com/ticket/11974
  • https://github.com/twbs/bootstrap/issues/20184
  • https://github.com/jquery/jquery/issues/2432

My first question would be, are Plone sites vulnerable to attacks using this? I am inclined to answering no, being that there is XSS protection at the server level, but I don't know for sure.

If this affects Plone, should we consider updating jQuery/Bootstrap to patched versions? I mean, we will be updating to newer versions I guess at some point, but if this is affecting security, we might need to do it ASAP.

Thoughts? /cc @thet @petschki @datakurre @sunew @plone/security-team

frapell avatar Sep 13 '18 18:09 frapell

You will also see these warnings when running :

..../plone.coredev-5.1/src/mockup (master)$ make bootstrap
mkdir -p ./mockup/build
npm link

audited 2066 packages in 4.152s
found 50 vulnerabilities (19 low, 26 moderate, 4 high, 1 critical)
  run `npm audit fix` to fix them, or `npm audit` for details

and

..../plone/plone.coredev-5.1 (fix-omelette-5.1)$ ./bin/plone-compile-resources
Setup npm env
Running command: npm install
audited 472 packages in 2.173s
found 7 vulnerabilities (2 low, 4 moderate, 1 high)
  run `npm audit fix` to fix them, or `npm audit` for details

Most of these are for the build tools. But we also make it possible to build less TTW.

I think we should upgrade parts of these, and start with identifying which ones - but it is not necessarily easy - jquery, bootstrap.. opens a can of worms:)

sunew avatar Sep 13 '18 18:09 sunew

The bootstrap issue is not fixed in a (v3) release yet: https://github.com/twbs/bootstrap/issues/25679

sunew avatar Sep 13 '18 18:09 sunew

Maybe we should just verify we're even affected? I imagine it only matters what features we utilize right?

On Thu, Sep 13, 2018 at 2:55 PM Sune Broendum Woeller < [email protected]> wrote:

The bootstrap issue is not fixed in a (v3) release yet: twbs/bootstrap#25679 https://github.com/twbs/bootstrap/issues/25679

— You are receiving this because you are on a team that was mentioned. Reply to this email directly, view it on GitHub https://github.com/plone/mockup/issues/865#issuecomment-421114422, or mute the thread https://github.com/notifications/unsubscribe-auth/AAgAfTonT0ezFOLDF_YtcQYUiIZA-0TFks5uaqoigaJpZM4Wn9bV .

vangheem avatar Sep 13 '18 18:09 vangheem

@vangheem for bootstrap at least I agree. But there is also some value in being able to report to clients what versions are depended upon; to be able to document that we are not using releases with security problems - even if we do not use the flawed features.

sunew avatar Sep 14 '18 07:09 sunew

I'm looking at some low hanging fruits to upgrade.

Question: Are we still using saucelabs? And are we still testing on these old browsers?

src/mockup/mockup/js/grunt.js:

          testCI: {
            singleRun: true,
            port: 8080,
            recordVideo: true,
            reporters: ['junit', 'coverage', 'saucelabs'],
            junitReporter: { outputFile: 'test-results.xml' },
            sauceLabs: { testName: 'Mockup', startConnect: true },
            browsers: BROWSERS,
            customLaunchers: {
              'SL_Chrome': { base: 'SauceLabs', browserName: 'chrome', platform: 'Windows 8.1', version: '38' },
              'SL_Firefox': { base: 'SauceLabs', browserName: 'firefox', platform: 'Windows 8.1', version: '33' },
              'SL_Opera': { base: 'SauceLabs', browserName: 'opera', platform: 'Windows 8.1', version: '25' },
              'SL_Safari': { base: 'SauceLabs', browserName: 'safari', platform: 'Mac 10.9', version: '7.1' },
              'SL_IE_8': { base: 'SauceLabs', browserName: 'internet explorer', platform: 'Windows 7', version: '8' },
              'SL_IE_9': { base: 'SauceLabs', browserName: 'internet explorer', platform: 'Windows 7', version: '9' },
              'SL_IE_10': { base: 'SauceLabs', browserName: 'internet explorer', platform: 'Windows 7', version: '10' },
              'SL_IE_11': { base: 'SauceLabs', browserName: 'internet explorer', platform: 'Windows 8.1', version: '11' },
              'SL_IPhone': { base: 'SauceLabs', browserName: 'iphone', platform: 'OS X 10.9', version: '7.1' },
              'SL_IPad': { base: 'SauceLabs', browserName: 'ipad', platform: 'OS X 10.9', version: '7.1' },
              'SL_Android': { base: 'SauceLabs', browserName: 'android', platform: 'Linux', version: '4.3' }
            }
          }
        },

sunew avatar Sep 15 '18 10:09 sunew

We are using a git checkout of grunt-sed on a specific hash. But the original and the fork are identical:

https://github.com/jharding/grunt-sed/compare/master...collective:master

I'd use the released version of grunt-sed instead

What is the rationale behind this? Historical reasons?

Ping @gforcada - I can see its your commit - do you remember what the reason was?

sunew avatar Sep 15 '18 11:09 sunew

Found the reason in the corresponding commit in cmfplone:

" Use fork of grunt-sed
The problem with released versions is that they are not compatible with newer grunt versions. This grunt-sed fork in collective only differs from latest official release with the grunt version fixes. There's no new/removed functionality. "

sunew avatar Sep 15 '18 11:09 sunew

I took care of some low-hanging fruits in #870 Things that should have no effect on the plone side, only build tools and cleanup.

sunew avatar Sep 18 '18 18:09 sunew

@sunew sorry I totally missed your question to me :sweat_smile: I guess that was the time I was adding the jenkins job to run mockup tests on jenkins.plone.org and somewhat official grunt-sed . On the pull request I was mentioning some dependency problems https://github.com/plone/mockup/pull/702

If it is already fixed, then forget about it.

gforcada avatar Nov 23 '18 17:11 gforcada

I don’t want to overreact...but maybe in future email [email protected] with security related questions or concerns...

tkimnguyen avatar Nov 23 '18 18:11 tkimnguyen

@tkimnguyen I think it's ok to discuss these issues here. Even github reports security alerts prominently at the repositories front page if it's using package.json (see: https://github.com/plone/plone.staticresources )

@sune not sure about saucelabs... But we don't need to test below Windows 10 and IE 11.

Bootstrap minor upgrades should be safe, jQuery major upgrades also.

thet avatar Nov 23 '18 19:11 thet

@thet note that only if you are an admin of the org you see those alerts (AFAIK)

gforcada avatar Nov 23 '18 20:11 gforcada

@frapell Can we close this issue now?

vincentfretin avatar Jun 27 '20 16:06 vincentfretin

@vincentfretin I believe the jQuery issue was fixed in https://github.com/plone/plone.staticresources/pull/70 I don't know about bootstrap though, but if we are using an unnafected version, I guess it can be closed...

frapell avatar Jun 29 '20 14:06 frapell