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

Using CSP breaks IGV.js #1613

Open
MarvinDo opened this issue Mar 22, 2023 · 27 comments
Open

Using CSP breaks IGV.js #1613

MarvinDo opened this issue Mar 22, 2023 · 27 comments
Assignees
Milestone

Comments

@MarvinDo
Copy link

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Author

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
Copy link
Contributor

@MarvinDo OK, thanks for the info!

@lacek
Copy link

lacek commented Oct 26, 2023

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.

@jrobinso jrobinso reopened this Oct 26, 2023
@jrobinso jrobinso self-assigned this Oct 26, 2023
@cmdcolin
Copy link

cmdcolin commented Jul 31, 2024

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

@jrobinso
Copy link
Contributor

@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
Copy link

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
Copy link
Contributor

@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
Copy link

cmdcolin commented Jul 31, 2024

"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 jrobinso modified the milestones: 3.0, 3.1 Jul 31, 2024
@jrobinso
Copy link
Contributor

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
Copy link
Contributor

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
Copy link

cmdcolin commented Jul 31, 2024

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 GMOD/binary-parser@ddc7a7a removal of the vm-browser.js file removed the eval)

@jrobinso
Copy link
Contributor

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

@cmdcolin
Copy link

cmdcolin commented Aug 1, 2024

went ahead and tried to get it done GMOD/cram-js#137

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

cram-bundle.js.gz

@jrobinso
Copy link
Contributor

jrobinso commented Aug 1, 2024

@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
Copy link

cmdcolin commented Aug 1, 2024

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
Copy link
Contributor

jrobinso commented Aug 1, 2024

@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
Copy link

cmdcolin commented Aug 2, 2024

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
Copy link

cmdcolin commented Aug 2, 2024

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

cram-bundle.js.gz

@jrobinso
Copy link
Contributor

jrobinso commented Aug 2, 2024

@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
Copy link
Contributor

jrobinso commented Aug 2, 2024

@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
Copy link
Contributor

jrobinso commented Aug 2, 2024

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
Copy link
Contributor

jrobinso commented Aug 2, 2024

@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
Copy link

cmdcolin commented Aug 2, 2024

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
Copy link
Contributor

The "eval" issue should be resolved.

@jrobinso
Copy link
Contributor

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants