plone / mockup

A collection of client side patterns for faster and easier web development
http://plone.github.io/mockup/
BSD 3-Clause "New" or "Revised" License
47 stars 93 forks source link

JS Libraries with XSS vulnerabilities #865

Open frapell opened 6 years ago

frapell commented 6 years ago

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:

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

sunew commented 6 years ago

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 commented 6 years ago

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

vangheem commented 6 years ago

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 < notifications@github.com> 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 .

sunew commented 6 years ago

@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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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.

gforcada commented 5 years ago

@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.

tkimnguyen commented 5 years ago

I don’t want to overreact...but maybe in future email security@plone.org with security related questions or concerns...

thet commented 5 years ago

@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.

gforcada commented 5 years ago

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

vincentfretin commented 4 years ago

@frapell Can we close this issue now?

frapell commented 4 years ago

@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...