Skip to content

Conversation

jorgectf
Copy link
Contributor

This PR adds modeling for the Webix framework.

cc @Kwstubbs

Co-authored-by: Kevin Stubbings <Kwstubbs@users.noreply.github.com>
@github-actions github-actions bot added the JS label Jun 21, 2023
@jorgectf jorgectf marked this pull request as ready for review June 23, 2023 16:06
@jorgectf jorgectf requested a review from a team as a code owner June 23, 2023 16:06
@calumgrant calumgrant requested a review from asgerf June 26, 2023 08:49
@asgerf
Copy link
Contributor

asgerf commented Jun 26, 2023

LGTM, just need an update to the test expectations for HeuristicSourceCodeInjection.ql, which resides in the same folder as the regular code injection test.

Copy link
Contributor

@asgerf asgerf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some optional comments for using API graphs instead of local data flow. Note that they're written off the top of the my head and I may have made mistakes.

@jorgectf
Copy link
Contributor Author

Some optional comments for using API graphs instead of local data flow. Note that they're written off the top of the my head and I may have made mistakes.

Thanks @asgerf! Done ✅

@Kwstubbs
Copy link
Contributor

Kwstubbs commented Jun 26, 2023

@asgerf I noticed that webix could be used in the form of

<script src="../../codebase/webix.js" type="text/javascript" charset="utf-8"></script>
<script>webix.extend(object1, object_with_usercontrolled_values) </script> 

Would CodeQL be able to identify calls inside script tags (within an HTML file)? We would like to support this use case as well if possible. Thanks!

Copy link
Contributor

@asgerf asgerf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would CodeQL be able to identify calls inside script tags (within an HTML file)? We would like to support this use case as well if possible. Thanks!

Yes, we'd usually use DataFlow::globalVarRef("webix") to find uses of the global variable webix, see the inline suggestion.

It's slightly cumbersome to get that working with API graphs (we're working on simplifying it). A good example of how to do this can be found in the D3 model:

result = any(D3GlobalEntry i).getANode()

@jorgectf
Copy link
Contributor Author

Nice catch @Kwstubbs, I have adapted the modeling as per #13529 (review) @asgerf suggestion

@jorgectf jorgectf requested a review from asgerf June 28, 2023 13:28
Copy link
Contributor

@asgerf asgerf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One final comment. I'll start an evaluation of the PR in the meantime.

Co-authored-by: Asger F <asgerf@github.com>
@asgerf asgerf merged commit 4c9501e into github:main Jul 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants