Support Trusted Types from DOM lib about tsec HOT 22 CLOSED

@tosmolka We were indeed talking about union types. The new behavior is that, if tsec found the definition of Trusted Types is ambient, union types like string|TrustedScriptURL are not longer acceptable at the sinks, since that may cause problems. For consistency, the following case will be flagged as well:

declare const trustedScriptUrl: string | TrustedScriptURL;
script.src = trustedScriptUrl as string;

On unit tests: we do have them internally, but for some reason we don't export the test code to github.

from tsec.

Thanks for reporting this. Due to internal issues, tsec can't accept external contributions (as stated in our CONTRIBUTING.md). I will patch this internally and release a new version.

from tsec.

Could you provide some details about your setup? Did you swap out the original lib.dom.d.ts and use an updated one? Or is the updated lib.dom.d.ts released as an npm package?

from tsec.

@uraj , I ended up patching lib.dom.d.ts in my local setup, I pushed patch to 27256dc as a reference.

from tsec.

@uraj , do you have enough info to investigate this further? Any updates? Thanks.

from tsec.

@tosmolka Sorry we're currently working on adding ESLint support. It should be done by the end of this week. After that I will add support for TT in DOM lib.

Another thing that might be useful to know: in which file do you define the Trusted Types? Did you put the definition in lib.dom.d.ts or you have a separate file for Type declaration?

from tsec.

@tosmolka I'm about to release a patch. Before that, want to confirm with you on how you amended lib.dom.d.ts. Did you just append something like below to the file?

declare class TrustedHTML {
  //...
}

from tsec.

@uraj , I used this: 27256dc and applied patch using patch-package.

from tsec.

Should be fixed by 1ceed96. Will publish a new version soon.

from tsec.

Please try 0.2.4.

from tsec.

@uraj , I did a quick test and I think this needs little bit of more work. I tried with HTMLScriptElement.src sink and tsec still flags when I do assignments with string | TrustedScriptURL or TrustedScriptURL types.

I think I faced similar issues when I proposed the PR and I fixed that by adding isTrustedType and isTrustedTypeUnionWithString methods to is_trusted_type.ts.

from tsec.

@tosmolka OK I got what you meant. I will discuss with the team and see whether we should support this form of assignments. With 0.2.4 you can now at least write

declare const trustedScriptUrl: string | TrustedScriptURL;
script.src = trustedScriptUrl as string;

where TrustedScriptURL is defined in the patched lib.dom.d.ts.

from tsec.

@uraj , do you have any updates regarding support for this form of assignments? Thanks a lot.

declare const trustedScriptUrl: string | TrustedScriptURL;
script.src = trustedScriptUrl;

from tsec.

@tosmolka With lib.dom.d.ts patched, there is no longer the need to use string | TrustedScriptURL. I think we can support the following

declare const trustedScriptUrl: TrustedScriptURL;
script.src = trustedScriptUrl;

Is that OK to you?

from tsec.

I think we will continue using string | TrustedScriptURL in certain cases (e.g. Trusted Types are not enabled/supported).

Now, if tsec keeps flagging this as an issue because string MIGHT flow into this sink - that's understandable and we would have to deal with it on our side.

But then I'd like tsec to be consistent and start flagging also this case:

declare const trustedScriptUrl: string | TrustedScriptURL;
script.src = trustedScriptUrl as string;

WDYT?

from tsec.

We can flag that case, if we detect that TrustedScriptURL is defined as an ambient type, i.e., an indicator that lib.dom.d.ts is patched. We should really check if script.src actually accepts an TrustedScriptURL but that seems a much more complicated change.

from tsec.

@tosmolka I've submitted the patch, but for some reason I can't publish a new version right now. I will try over the weekends, but you can try out the patch before that if you want

from tsec.

0.2.6 is now published. Let me know if the new version doesn't work for your setup.

from tsec.

@uraj , I quickly checked the change.

It seems to be working fine for intersection types - string & TrustedScriptURL . But what we need are union types - string | TrustedScriptURL - this is still being flagged by tsec as an issue.

from tsec.

@tosmolka this is intended. I thought your project is OK with this, per our previous discussions.

from tsec.

@uraj , I thought we were discussion union types - string | TrustedScriptURL - this is what we are using in our projects, this is what I . I am confused now, are you saying we should change our setup or are you saying you are going to fix this in tsec? Can you add unit tests for these cases into tsec so that the expected behavior is clearer? Thanks.

from tsec.

@uraj , OK, I misunderstood what is the intended behavior after your fix. Would be good to update docs accordingly at some point. Thanks.

from tsec.