EarthSystemCoG / COG

COG source code
BSD 3-Clause "New" or "Revised" License
8 stars 16 forks source link

Prevent CoG pages from being included in IFRAMEs #1314

Closed LucaCinquini closed 8 years ago

LucaCinquini commented 8 years ago

Who: NOAA security review

The NOAA security review has identified a potential XSS vulnerability since CoG pages can potentially be included in IFRAME tags on other sites.

LucaCinquini commented 8 years ago

Following suggestion, executed 2 actions:

o CoG pages now set the header X_FRAME_OPTIONS = 'DENY' that is respected by modern browsers to prevent pages to be embedded in IFRAMEs

o Also, a little piece of JavaScript code in "layout_base.html" will protect older browsers, since the embedded page will be reloaded as the top level page.

murphysj commented 8 years ago

I assume this does not affect other pages from being embedded in cog?

On Wed, Aug 24, 2016 at 10:22 AM, Luca Cinquini notifications@github.com wrote:

Closed #1314 https://github.com/EarthSystemCoG/COG/issues/1314.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/EarthSystemCoG/COG/issues/1314#event-766241874, or mute the thread https://github.com/notifications/unsubscribe-auth/ABcVAIsxyP88u9eyihPt6FC87yUoQluwks5qjG_NgaJpZM4JsMUI .


Sylvia Murphy NESII/CIRES/NOAA Earth System Research Laboratory 325 Broadway, Boulder CO 80305 Time Zone: U.S. Mountain Web: http://www.esrl.noaa.gov/nesii/ Email: sylvia.murphy@noaa.gov Phone: 303-497-7753

LucaCinquini commented 8 years ago

Probably not but it is a bad practice anyway… IFRAME should be eliminated… L

On Aug 24, 2016, at 10:44 AM, Sylvia Murphy notifications@github.com wrote:

I assume this does not affect other pages from being embedded in cog?

On Wed, Aug 24, 2016 at 10:22 AM, Luca Cinquini notifications@github.com wrote:

Closed #1314 https://github.com/EarthSystemCoG/COG/issues/1314.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/EarthSystemCoG/COG/issues/1314#event-766241874, or mute the thread https://github.com/notifications/unsubscribe-auth/ABcVAIsxyP88u9eyihPt6FC87yUoQluwks5qjG_NgaJpZM4JsMUI .


Sylvia Murphy NESII/CIRES/NOAA Earth System Research Laboratory 325 Broadway, Boulder CO 80305 Time Zone: U.S. Mountain Web: http://www.esrl.noaa.gov/nesii/ Email: sylvia.murphy@noaa.gov Phone: 303-497-7753 — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/EarthSystemCoG/COG/issues/1314#issuecomment-242131897, or mute the thread https://github.com/notifications/unsubscribe-auth/AAm4g50sFa5pZgseCZpwE4xgPb2UFj15ks5qjHTsgaJpZM4JsMUI.

murphysj commented 8 years ago

maybe so, but it is one of the critical functions for the boss.

On Wed, Aug 24, 2016 at 10:47 AM, Luca Cinquini notifications@github.com wrote:

Probably not but it is a bad practice anyway… IFRAME should be eliminated… L

On Aug 24, 2016, at 10:44 AM, Sylvia Murphy notifications@github.com wrote:

I assume this does not affect other pages from being embedded in cog?

On Wed, Aug 24, 2016 at 10:22 AM, Luca Cinquini < notifications@github.com> wrote:

Closed #1314 https://github.com/EarthSystemCoG/COG/issues/1314.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/EarthSystemCoG/COG/issues/1314#event-766241874, or mute the thread https://github.com/notifications/unsubscribe-auth/ ABcVAIsxyP88u9eyihPt6FC87yUoQluwks5qjG_NgaJpZM4JsMUI .


Sylvia Murphy NESII/CIRES/NOAA Earth System Research Laboratory 325 Broadway, Boulder CO 80305 Time Zone: U.S. Mountain Web: http://www.esrl.noaa.gov/nesii/ Email: sylvia.murphy@noaa.gov Phone: 303-497-7753 — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/ EarthSystemCoG/COG/issues/1314#issuecomment-242131897, or mute the thread https://github.com/notifications/unsubscribe-auth/ AAm4g50sFa5pZgseCZpwE4xgPb2UFj15ks5qjHTsgaJpZM4JsMUI.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/EarthSystemCoG/COG/issues/1314#issuecomment-242132869, or mute the thread https://github.com/notifications/unsubscribe-auth/ABcVAPXAc_bnuEMaFZflg1sRS90dkcKFks5qjHW5gaJpZM4JsMUI .


Sylvia Murphy NESII/CIRES/NOAA Earth System Research Laboratory 325 Broadway, Boulder CO 80305 Time Zone: U.S. Mountain Web: http://www.esrl.noaa.gov/nesii/ Email: sylvia.murphy@noaa.gov Phone: 303-497-7753

LucaCinquini commented 8 years ago

Let’s discuss this later today L

On Aug 24, 2016, at 10:51 AM, Sylvia Murphy notifications@github.com wrote:

maybe so, but it is one of the critical functions for the boss.

On Wed, Aug 24, 2016 at 10:47 AM, Luca Cinquini notifications@github.com wrote:

Probably not but it is a bad practice anyway… IFRAME should be eliminated… L

On Aug 24, 2016, at 10:44 AM, Sylvia Murphy notifications@github.com wrote:

I assume this does not affect other pages from being embedded in cog?

On Wed, Aug 24, 2016 at 10:22 AM, Luca Cinquini < notifications@github.com> wrote:

Closed #1314 https://github.com/EarthSystemCoG/COG/issues/1314.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/EarthSystemCoG/COG/issues/1314#event-766241874, or mute the thread https://github.com/notifications/unsubscribe-auth/ ABcVAIsxyP88u9eyihPt6FC87yUoQluwks5qjG_NgaJpZM4JsMUI .


Sylvia Murphy NESII/CIRES/NOAA Earth System Research Laboratory 325 Broadway, Boulder CO 80305 Time Zone: U.S. Mountain Web: http://www.esrl.noaa.gov/nesii/ Email: sylvia.murphy@noaa.gov Phone: 303-497-7753 — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/ EarthSystemCoG/COG/issues/1314#issuecomment-242131897, or mute the thread https://github.com/notifications/unsubscribe-auth/ AAm4g50sFa5pZgseCZpwE4xgPb2UFj15ks5qjHTsgaJpZM4JsMUI.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/EarthSystemCoG/COG/issues/1314#issuecomment-242132869, or mute the thread https://github.com/notifications/unsubscribe-auth/ABcVAPXAc_bnuEMaFZflg1sRS90dkcKFks5qjHW5gaJpZM4JsMUI .


Sylvia Murphy NESII/CIRES/NOAA Earth System Research Laboratory 325 Broadway, Boulder CO 80305 Time Zone: U.S. Mountain Web: http://www.esrl.noaa.gov/nesii/ Email: sylvia.murphy@noaa.gov Phone: 303-497-7753 — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/EarthSystemCoG/COG/issues/1314#issuecomment-242133969, or mute the thread https://github.com/notifications/unsubscribe-auth/AAm4g4ap3bO61QDQ8EtiTH1uzpIqN0hlks5qjHaLgaJpZM4JsMUI.

LucaCinquini commented 7 years ago

It turns out that I had to back out of two changes:

o X_FRAME_OPTIONS = 'DENY' will break image upload with CKEditor, so I restored the default X_FRAME_OPTIONS = 'SAMEORIGIN'

o The JS code in layout_base.html to bust out of frames also breaks image upload, so I removed it.