Skip to content

Commit

Permalink
ROU-11001: Added sanitizeInputValues configuration (#431)
Browse files Browse the repository at this point in the history
This PR is for adding the new `sanitizeInputValues` configuration.

### What was happening
* When passing HTML to the Grid, mainly on Action and Image Columns and
the ContextMenu., we opened a door to an XSS vulnerability.

### What was done
* Added a new `sanitizeInputValues` configuration to allow the developer
to control when to sanitize the Grid's data.
* The default will be `sanitizeInputValues = true` so this will be a
needed **breaking change**.

### Test Steps
1. Open a screen with a Link Column 
2. On the Get From Other Sources add to the first position the following
text `Test <img src=x onerror='alert(String.fromCharCode(88,83,83))'>`
3. Check that now we can see the text and no code is executed


### Screenshots
- Before:


![image](https://github.com/user-attachments/assets/9d7f7f2c-fd05-45ac-bf6b-ed83651ffee3)


- After the fix:


![image](https://github.com/user-attachments/assets/a4a53714-75b2-48b2-b1cc-4ddb26e7a5ff)


### Checklist
* [X] tested locally
* [X] documented the code
* [X] clean all warnings and errors of eslint
* [X] requires changes in OutSystems 
* [ ] requires new sample page in OutSystems
  • Loading branch information
gnbm authored Aug 21, 2024
2 parents 026f095 + 1156f28 commit 0844059
Show file tree
Hide file tree
Showing 7 changed files with 32 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ namespace OSFramework.DataGrid.Configuration.Grid {
public rowHeader: Enum.RowHeader;
public rowHeight: number;
public rowsPerPage: number;
public sanitizeInputValues: boolean;
public selectionMode: number;
public serverSidePagination: boolean;
public showAggregateValues: boolean;
Expand Down
4 changes: 4 additions & 0 deletions src/OSFramework/DataGrid/Configuration/IConfigurationGrid.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ namespace OSFramework.DataGrid.Configuration {
*/
keyBinding: string;
/**
* Indicates if the grid should sanitize the input values or not
*/
sanitizeInputValues: boolean;
/**
Indicates if the grid is in server side pagination mode
*/
serverSidePagination: boolean;
Expand Down
4 changes: 4 additions & 0 deletions src/OutSystems/GridAPI/GridManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ namespace OutSystems.GridAPI.GridManager {
let output = false;
if (grid !== undefined) {
if (grid.isReady && data !== '' && data !== '{}') {
// When the configurantion is set to sanitize the input values, we need to sanitize the data before setting it
if (grid.config.sanitizeInputValues) {
data = OSFramework.DataGrid.Helper.Sanitize(data);
}
grid.setData(data);
}
output = true;
Expand Down
3 changes: 2 additions & 1 deletion src/Providers/DataGrid/Wijmo/Columns/ActionColumn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ namespace Providers.DataGrid.Wijmo.Column {
config.binding,
this.handleActionEvent.bind(this),
undefined,
this.config.externalURL
this.config.externalURL,
this.grid.config.sanitizeInputValues
);

return config;
Expand Down
4 changes: 3 additions & 1 deletion src/Providers/DataGrid/Wijmo/Columns/ImageColumn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@ namespace Providers.DataGrid.Wijmo.Column {
this.config.actionColumnElementType,
config.binding,
this.handleActionEvent.bind(this),
this.config.altText
this.config.altText,
undefined /* externalURL */,
this.grid.config.sanitizeInputValues
);

return config;
Expand Down
11 changes: 8 additions & 3 deletions src/Providers/DataGrid/Wijmo/Features/ContextMenu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -313,8 +313,8 @@ namespace Providers.DataGrid.Wijmo.Feature {
executeCommand: OSFramework.DataGrid.Callbacks.ContextMenu.OSClickEvent
): void {
const menuItem = new OSFramework.DataGrid.Feature.Auxiliar.MenuItem(menuItemId);

menuItem.label = label;
// Sanitize the label if the configuration is set to do so
menuItem.label = this.grid.config.sanitizeInputValues ? OSFramework.DataGrid.Helper.Sanitize(label) : label;
menuItem.enabled = enabled;
menuItem.clickEvent = executeCommand;

Expand All @@ -339,7 +339,12 @@ namespace Providers.DataGrid.Wijmo.Feature {
const menuItem = this._menuItems.get(menuItemId);
if (menuItem) {
if (menuItem.hasOwnProperty(propertyName)) {
menuItem[propertyName] = propertyValue;
if (propertyName === 'label' && this.grid.config.sanitizeInputValues) {
// Sanitize the label if the configuration is set to do so
menuItem.label = OSFramework.DataGrid.Helper.Sanitize(propertyValue as string);
} else {
menuItem[propertyName] = propertyValue;
}
} else {
console.error(`MenuItem "${menuItem.label}" has no property "${propertyName}" defined.`);
}
Expand Down
12 changes: 10 additions & 2 deletions src/Providers/DataGrid/Wijmo/Helper/CellTemplateFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,27 @@ namespace Providers.DataGrid.Wijmo.Helper.CellTemplateFactory {
binding: string,
callback: (item) => void,
altText?: string,
externalURL?: string
externalURL?: string,
sanitizeInputValues?: boolean
): wijmo.grid.ICellTemplateFunction {
let cellTemplate: wijmo.grid.ICellTemplateFunction;

const hasFixedText = binding.startsWith('$');
const hasExternalURL = externalURL?.toLocaleLowerCase().startsWith('http');

const url = hasExternalURL ? externalURL : '${item.' + externalURL + '}';
const text = hasFixedText ? binding.substring(1) : undefined;
let text = hasFixedText ? binding.substring(1) : undefined;

// Sanitize the text if the configuration is set to do so
if (text !== undefined) {
text = sanitizeInputValues ? OSFramework.DataGrid.Helper.Sanitize(text) : text;
}

let imgAltText = '';
if (altText !== undefined) {
const hasFixedAltText = altText.startsWith('$');
// Sanitize the alternative text if the configuration is set to do so
altText = sanitizeInputValues ? OSFramework.DataGrid.Helper.Sanitize(altText) : altText;
imgAltText = hasFixedAltText ? altText.substring(1) : '${item.' + altText + '}';
}

Expand Down

0 comments on commit 0844059

Please sign in to comment.