Skip to content

Conversation

mbaluda
Copy link
Contributor

@mbaluda mbaluda commented Aug 29, 2023

This PR adds support for dataflow through event handlers and their parameters, as well as a few other fixes as follows:

Support dataflow through event handlers Fix /issues/24

  • cherry-pick modelling from Cover various event handlers #28
  • Implement class UI5Handler and ControlTypeInHandlerModel to model references to Controls inside the handler Javascript code (e.g. oEvent.getSource() this.getView().byId("id")) (see tests doSomething3 and doSomething4)
  • bindingPathCapture now covers event handler parameters syntax like.doSomething(${/input}) (limited to an infividual call with a single argument)
  • isAdditionalFlowStep includes edge from UI5BindingPath to handler when passed as parameter (see test doSomething2)
  • isAdditionalFlowStep includes edge from UI5BindingPath to handler when passed as parameter (see test doSomething2)
  • PathGraph includes edges through XML (js->xml->js) to show handler parameters flow
  • A large part of the dataflow configuration is now shared between the dataflow queries

Varia

  • Improves the workflow scripts with better naming and error messages
  • Modify data extension typeModel section so that types are selected as well as their instances (this is to avoid the need for Instance.Member in sourceModel entries)
  • Restores Renderer definition that was broken in MaD and remove unneeded RenderManager class and getRenderer() predicate
  • Remove redundant UnsafeHtmlXssSource and UnsafeHtmlXssSink classes
  • move test xss-example from integration-test folder to test
  • Fix writeAttributeEscaped erroneously included in SinkModel #34

@github-advanced-security
Copy link

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

<Button text="Press Me XSS2" press=".doSomething2(${/input})"/>
<core:HTML content="{/output2}"/> <!--XSS sink sap.ui.core.HTML.content -->
<Button text="Press Me XSS1" press=".doSomething1" />
<core:HTML content="{/output1}" /> <!--XSS sink sap.ui.core.HTML.content -->

Check warning

Code scanning / CodeQL

Client-side cross-site scripting

XSS vulnerability due to [user-provided value](1).
liveChange=".doSomething3" />
<core:HTML content="{/output3}"/> <!--XSS sink sap.ui.core.HTML.content -->
<Button text="Press Me XSS2" press=".doSomething2(${/input})" />
<core:HTML content="{/output2}" /> <!--XSS sink sap.ui.core.HTML.content -->

Check warning

Code scanning / CodeQL

Client-side cross-site scripting

XSS vulnerability due to [user-provided value](1). XSS vulnerability due to [user-provided value](2). XSS vulnerability due to [user-provided value](3). XSS vulnerability due to [user-provided value](4). XSS vulnerability due to [user-provided value](5). XSS vulnerability due to [user-provided value](6). XSS vulnerability due to [user-provided value](7). XSS vulnerability due to [user-provided value](8). XSS vulnerability due to [user-provided value](9). XSS vulnerability due to [user-provided value](10). XSS vulnerability due to [user-provided value](11). XSS vulnerability due to [user-provided value](12). XSS vulnerability due to [user-provided value](13). XSS vulnerability due to [user-provided value](14). XSS vulnerability due to [user-provided value](15). XSS vulnerability due to [user-provided value](16). XSS vulnerability due to [user-provided value](17). XSS vulnerability due to [user-provided value](18). XSS vulnerability due to [user-provided value](19).
@mbaluda mbaluda changed the title Mbaluda handlers Track dataflow through event handlers and their parameters Sep 1, 2023
@mbaluda
Copy link
Contributor Author

mbaluda commented Sep 1, 2023

Tests doSomething3 and doSomething4 are now supported!
There is however a bug in CodeQL (see PR github/codeql#14120) that can luckily be worked around

<Input placeholder="Enter Payload"
description="Try: &lt;img src=x onerror=alert(&quot;XSS&quot;)&gt;"
liveChange=".doSomething3" />
<core:HTML content="{/output3}" /> <!--XSS sink sap.ui.core.HTML.content -->

Check warning

Code scanning / CodeQL

Client-side cross-site scripting

XSS vulnerability due to [user-provided value](1).
var oHtmlOutput = oView.byId("htmlOutput");

var value = oInput.getValue() // User input source sap.m.Input#getValue
oHtmlOutput.setContent(value) // XSS sink sap.ui.core.HTML#setContent

Check warning

Code scanning / CodeQL

Client-side cross-site scripting

XSS vulnerability due to [user-provided value](1).
@mbaluda mbaluda marked this pull request as ready for review September 5, 2023 01:28
@jeongsoolee09
Copy link
Contributor

Left some partial comments above.

@jeongsoolee09
Copy link
Contributor

Submitted another batch of comments.

Copy link
Contributor

@jeongsoolee09 jeongsoolee09 left a comment

Choose a reason for hiding this comment

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

Thank you a gigaton, @mbaluda ! It's truly a magnificent work.

@jeongsoolee09 jeongsoolee09 merged commit a38290b into main Sep 7, 2023
@jeongsoolee09 jeongsoolee09 deleted the mbaluda-handlers branch January 26, 2024 23:09
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

Successfully merging this pull request may close these issues.

writeAttributeEscaped erroneously included in SinkModel UI5 support flow through event handlers
2 participants