search

igvteam / igv.js

Embeddable genomic visualization component based on the Integrative Genomics Viewer
MIT License
643 stars 229 forks source link

Using CSP breaks IGV.js #1613

Open MarvinDo opened 1 year ago

MarvinDo commented 1 year ago

Hello everyone,

recently I started experimenting with CSP. When enabled using the "Content-Security-Policy" http header with these options: default-src 'self' 'report-sample'; the igv.min.js throws a couple of errors:

Uncaught EvalError: call to Function() blocked by CSP
    _p http://website/static/packages/igv/igv.min.js:51
    n http://website/static/packages/igv/igv.min.js:38
    _p http://website/static/packages/igv/igv.min.js:45
    n http://website/static/packages/igv/igv.min.js:38
    _p http://website/static/packages/igv/igv.min.js:51
    _p http://website/static/packages/igv/igv.min.js:51
    n http://website/static/packages/igv/igv.min.js:38
    _p http://website/static/packages/igv/igv.min.js:45
    n http://website/static/packages/igv/igv.min.js:38
    _p http://website/static/packages/igv/igv.min.js:51
    n http://website/static/packages/igv/igv.min.js:38
    _p http://website/static/packages/igv/igv.min.js:38
    <anonymous> http://website/static/packages/igv/igv.min.js:38
    <anonymous> http://website/static/packages/igv/igv.min.js:1
    <anonymous> http://website/static/packages/igv/igv.min.js:1

(source location: [igv.min.js:51:45899])

Content Security Policy: The page settings have blocked loading a resource on inline ("default-src"). Source: .igv-ui-1_3_0-popover {
  cursor: default;
  position: absolute;
  z-index: 2048;
  border-color: #7F7F7F;
  border-radius: 4px;
  border-style: solid;
  border-width: 1px;
  font-family: "Open Sans", sans-serif;
  font-size: small;
  background-color: white; }
  .igv-ui-1_3_0-popover > div:first-child {
    display: flex;
    flex-direction: row;
    flex-wrap: nowrap;
    justify-content: space-between;
    align-items: center;
    width: 100%;
    height: 24px;
    cursor: move;
    border-top-left-radius: 4px;
    border-top-right-radius: 4px;
    border-bottom-color: #7F7F7F;
    border-bottom-style: solid;
    border-bottom-width: thin;
    background-color: #eee; }
    .igv-ui-1_3_0-popover > div:first-child > div:first-child {
      margin-left: 4px; }
    .igv-ui-1_3_0-popover > div:first-child > div:last-child {
      margin-right: 4px;
      heig…

(source location: [igv.min.js:26:116108])

Content Security Policy: The page settings have blocked loading a resource on eval ("script-src"). Source: function anonymous(
) {
return this
}.

(source location: [igv.min.js:45:20173])

Content Security Policy: The page settings have blocked loading a resource on eval ("script-src"). Source: function anonymous(r
) {
regeneratorRuntime = r
}.

(source location: [igv.min.js:51:45898])

As a result igv is not loading on my webpage. Is it possible to fix these errors or can you help me with relaxing the CSP such that it works? I appreciate any help with this.

I am using IGV.js 2.13.9 and Firefox 102.8.0esr (64-Bit)

jrobinso commented 1 year ago

I don't know anything about CSP, but it looks like it is failing when trying to "eval" a runtime plugin for ES5 compatibility. If you are able to use the es6 package (import igv rather than a script include) you should not encounter this probability. Alternatively you could build you own version and remove the "babel" translation step. We are going to do this very soon in any event and drop support for ES5.

jrobinso commented 1 year ago

Also, that is not the latest version of igv.js, please try version 2.15.0. I don't think you should see this specific error in the latest version.

MarvinDo commented 1 year ago

Thank you for your reply. Here is what I did: I updated to 2.15.0 and changed to the ES6 package. However, I still got some errors. Most of them were complaining about the inline CSS parts (similar to the second error from the initial question). I found two possible ways to fix this. Either you build your own version and not include CSS (-> load a separate CSS file in your html containing all the neccessary styles) or allow inline styles using style-src 'unsafe-inline'; in your CSP header. Next, there was one more error left complaining about WebAssembly execution. I believe the gmod/cram package used in igv.js causes this to happen. If you do not need to load cram files you can simply remove the dependency. If you do need it I believe you must add script-src 'wasm-unsafe-eval'; to your CSP header. I also disabled the default genomes part, but you can also tell CSP to allow the two default genome resources explicitly.

To sum it all up: If you want very restrictive CSP you should build your own version. However, it is also possible to relax the CSP such that it works without changing the code.

jrobinso commented 1 year ago

@MarvinDo OK, thanks for the info!

lacek commented 1 year ago

I am encountering similar issue (with npm package igv version 2.15.11). I had to add the following to the CSP header to bypass the CSP restriction:

  1. style-src 'self' 'unsafe-inline'
  2. script-src 'self' 'unsafe-eval'

About unsafe-inline in style-src

Although the inclusion of unsafe-inline resolves the issue, it undermines CSP (e.g. as discussed in https://content-security-policy.com/unsafe-inline/ and https://stackoverflow.com/a/31759553/1089242). It is therefore not recommended in general by vulnerability scanners (e.g. https://www.zaproxy.org/docs/alerts/10055-6/).

I believe a way to eliminate the need for unsafe-inline in style-src would be to support the use of nonce in igv.js. One way is to add an option (e.g. cspNonce) to the igv.js browser class and add the attribute "nonce={cspNonce}" whenever a style element is created and cspNonce is provided (e.g. in https://github.com/igvteam/igv.js/blob/f330ef446f39a1ab444b6e6bec77cf619212af09/scripts/embedCssTemplate.js). Developer using igv.js can then opt in to provide such option when they implement CSP. For others who don't care about CSP, they can just ignore it.

About unsafe-eval in script-src

This is not recommended by vulnerability scanners too (e.g. https://www.zaproxy.org/docs/alerts/10055-10/). But I have no idea how to get rid of it in igv.js.

The CSP violation comes from the use of eval in https://github.com/igvteam/igv.js/blob/f330ef446f39a1ab444b6e6bec77cf619212af09/js/vendor/cram-bundle.js. However, eval cannot be found in the source code of cram-js (https://github.com/GMOD/cram-js/tree/v1.7.3). So it is likely that eval is introduced by the bundler or transpiler that creates cram-bundle.js.

cmdcolin commented 3 months ago

related to our use of the binary-parser library in CRAM (and other GMOD libraries...similar issue reported to the binary-parser library itself https://github.com/keichi/binary-parser/issues/258)

jrobinso commented 3 months ago

@cmdcolin Do you know what is inserting "eval" into the cram.js bundle? Is it webpack or babel, and is there a way to build the bundle without it?

cmdcolin commented 3 months ago

the binary-parser calls "new Function" which is effectively an eval

link into the binary-parser source code: https://github.com/keichi/binary-parser/blob/d0332454adff5ded936ce87f0397de11b1fd0709/lib/binary_parser.ts#L839-L847

probably it will be required to move away from the binary-parser library in cram-js to fix this. it can be done...just will take a bit of time

jrobinso commented 3 months ago

@cmdcolin "new Function" would likely be a security problem as well, but I am not convinced that is where the eval is coming from. "new Function" is perfectly fine javascript and would not need a transformation to eval. I know webpack uses eval in some of its transformations, that is one reason we dropped it for rollup.

I might take a crack at creating an eval free bundle, but I would like to start from javascript. I assume the "esm" folder that is created during the build is just the javascript generated from the typescript, otherwise unmodified. Is that correct?

cmdcolin commented 3 months ago

"new Function" is basically equivalent to an eval so it is the reason it warns to my knowledge, and since the warnings are pointing at the cram-bundle, that is more than likely the issue (the new Function from binary-parser in the cram-bundle)

since I stirred the pot by commenting on this issue, i started an attempt to purge binary-parser from cram-js

https://github.com/GMOD/cram-js/compare/master...no_binary_parser?expand=1

it is not done yet but it's on its way there

I might take a crack at creating an eval free bundle, but I would like to start from javascript. I assume the "esm" folder that is created during the build is just the javascript generated from the typescript, otherwise unmodified. Is that correct?

yes, it is created from running the typescript compiler over the src folder, without much down-transpilation. but i'd advise to wait to see if i can get my branch working...will try to get an update sometime soon

jrobinso commented 3 months ago

OK, will wait. I think it might be several days to a week of work for me to try. There is an "eval" in the cram bundle, you can verify that with grep. I can't tell if that is a consequence of "new Function" or not, but for sure "eval" was in every webpack bundle we ever created for igv.js. There might be options to create an ESM module from webpack that does not insert the eval, I haven't used webpack in many years.

jrobinso commented 3 months ago

WRT the injected CSS, which is the other issue noted here, (1) we now inject into a shadow dom, rather than the page "head" element. I don't know if this fixes the problem, I doubt it just noting it. (2) I will add a "no-css" bundle to the standard build, with instructions to included our CSS when using it.

cmdcolin commented 3 months ago

There is an "eval" in the cram bundle

yes I actually sort of skimmed over it but we actually use our own custom version of binary-parser named @gmod/binary-parser that forked off from binary-parser a long time ago, and that contained a usage of eval. i am actually happy to try to get rid of it as it would remove our need to maintain it :) note that the latest @gmod/binary-parser has no evals and just new Function but it would likely still trigger the same csp thing (see https://github.com/GMOD/binary-parser/commit/ddc7a7ab3e9de135fbbb5fe470e61141f802d20c removal of the vm-browser.js file removed the eval)

jrobinso commented 3 months ago

Ahh o.k. I also have a "binary parser" class from ages ago, but am slowly replacing it with DataView.

cmdcolin commented 3 months ago

went ahead and tried to get it done https://github.com/GMOD/cram-js/pull/137

you can manually clone branch, yarn, yarn build -> get dist/cram-bundle.js or i attached it here :)

cram-bundle.js.gz

jrobinso commented 3 months ago

@cmdcolin Wow thanks for the fast work! I did a quick test and get the following error. I can look into this more deeply tomorrow evening, I am out of the office most of the day tomorrow.

ReferenceError: Buffer is not defined at new BufferCache (http://localhost:63342/igv.js/js/cram/fileHandler.js:61:23) at new FileHandler (http://localhost:63342/igv.js/js/cram/fileHandler.js:15:26) at new CramReader (http://localhost:63342/igv.js/js/cram/cramReader.js:32:65) at new BamSource (http://localhost:63342/igv.js/js/bam/bamSource.js:58:30) at BAMTrack.init (http://localhost:63342/igv.js/js/bam/bamTrack.js:58:30) at new TrackBase (http://localhost:63342/igv.js/js/trackBase.js:64:14) at new BAMTrack (http://localhost:63342/igv.js/js/bam/bamTrack.js:52:9) at http://localhost:63342/igv.js/js/trackFactory.js:31:44 at getTrack (http://localhost:63342/igv.js/js/trackFactory.js:77:37) at Browser.createTrack (http://localhost:63342/igv.js/js/browser.js:1251:23)

cmdcolin commented 3 months ago

interesting, i think i fixed it in the cram-bundle with a webpack config tweak

re-attached here (and updated at pr)

cram-bundle.js.gz

jrobinso commented 3 months ago

@cmdcolin I still get the error but I'm considering in igv.js, not cram.js. The previous builds of cram.js apparently created a global Buffer class as a side effect, and I am using Buffer when creating a file handle for the cram library. I'm sure I can recode this without using Buffer.

cmdcolin commented 3 months ago

interesting, i could make a custom build of cram-bundle.js that sets window.Buffer=Buffer basically

the alternative is like you said, recoding it so your app doesn't depend on that dependency but if it's easier to just keep that behavior i can make that custom build to restore it

cmdcolin commented 3 months ago

here is that alternative build which sets window.Buffer=Buffer

cram-bundle.js.gz

jrobinso commented 3 months ago

@cmdcolin That works! I'm still going to try to rewrite this to use browser available classes, we shouldn't be depending on this side effect, but for now it works and no eval!

BTW, I do the following to convert the bundle to an es6 module. I don't mind doing it, but thought you might find it helpful if you need a browser friendly es6 module

To convert cram-bundle.js to an es6 module:

(1) Add an export default statment to the beginning of the file

 export default (() => {var e = {368....

(2) replace window.gmodCram=n at the end of the file with return n

var n = r(5590);return n})();
jrobinso commented 3 months ago

@cmdcolin I'm trying to rewrite my FileHandle class to not explicitly use Buffer. I can't strictly do this as some of the required return types are Buffer, but thought that maybe cram.js doesn't require the entire Buffer interface. Anyway, I found that the first argument to the "read" method, which I expect to be a Buffer base on the type script, is an Uint8Array in every case. This is actually fine, even preferred in a browser, but I'm a bit confused now on what to expect. Is the typescript too strict here, or an Uint8Array considered a Buffer?

Another oddity, I just realized the "read" method expects a return value, {bytesRead, buffer}, but I have never returned anything from my FileHandle and its been working. I assume the second argment, Buffer, can be whatever is passed in (an Uint8Array in my test cases), and not an actual Buffer.

export interface GenericFilehandle {
  read(
    buf: Buffer,
    offset: number,
    length: number,
    position: number,
    opts?: FilehandleOptions,
  ): Promise<{ bytesRead: number; buffer: Buffer }>
  readFile(): Promise<Buffer>
  readFile(options: BufferEncoding): Promise<string>
  readFile<T extends undefined>(
    options:
      | Omit<FilehandleOptions, 'encoding'>
      | (Omit<FilehandleOptions, 'encoding'> & { encoding: T }),
  ): Promise<Buffer>
  readFile<T extends BufferEncoding>(
    options: Omit<FilehandleOptions, 'encoding'> & { encoding: T },
  ): Promise<string>
  readFile<T extends BufferEncoding>(
    options: Omit<FilehandleOptions, 'encoding'> & { encoding?: T },
  ): T extends BufferEncoding ? Promise<Buffer> : Promise<Buffer | string>
  readFile(
    options?: FilehandleOptions | BufferEncoding,
  ): Promise<Buffer | string>
  stat(): Promise<Stats>
  close(): Promise<void>
}
jrobinso commented 3 months ago

Actually never mind re type, I think the Chrome developer tools are a bit confused and presenting this as an Uint8Array because a Buffer, I think, is a type of Uint8Array.

So it might be difficult to implement FileHandle using just browser native classes as the interface expects Buffer objects for both inputs and return values. Not a big deal, we have been doing this for years now.

jrobinso commented 3 months ago

@cmdcolin OK here's what I've done, and we can close this topic here or move it to JBrowse. I'm importing the "Buffer" project from https://www.npmjs.com/package/buffer and using that implementation, which is the same you use in cram.js. Its a bit redundant but what's an extra 60kb when your up to 3.3 MB. So you can remove the window.Buffer hack, I never should have used that in the first place. It just worked so I didn't question it.

cmdcolin commented 3 months ago

that is probably fine. the cram module uses that same one internally.

I have a long term goal to remove Buffer usage from our data parsing libraries eventually, but that will take awhile:)

jrobinso commented 1 week ago

The "eval" issue should be resolved.

jrobinso commented 1 week ago

@lacek Or anyone else, I'm willing to consider the "nonce" idea for CSS injection but am unsure on how to implement it. A PR or explicit instructions would be welcome. CSS injection occurs here: https://github.com/igvteam/igv.js/blob/f330ef446f39a1ab444b6e6bec77cf619212af09/scripts/embedCssTemplate.js