Sanitization in code

đź’¬ General Discussion
1

do anyone know how we can use DOMPurify method to sanitize code
I have to sanitize validation in this file code \include\InlineEditing\inlineEditing.js

if (html) {
                $(_this).html(
                    validation +
                        "<form name='EditView' id='EditView'><div id='inline_edit_field'>" +
                        html +
                        "</div><a id='inlineEditSaveButton'></a></form>"
                );
                $("#inlineEditSaveButton").html(
                    '<span class="suitepicon suitepicon-action-confirm"></span>'
                );
                //If the field is a relate field we will need to retrieve the extra js required to make the field work.
                if (type == "relate" || type == "parent") {
                    var relate_js = getRelateFieldJS(field, module, id);
                    $(_this).append(relate_js);
                    SUGAR.util.evalScript($(_this).html());
                    // Issue 2344 and 2499 changes - Dump existing QSProcessedFieldsArray to enable multiple QS on multiple rows.
                    var fieldToCheck = "EditView_" + field + "_display";
                    if (fieldToCheck in QSProcessedFieldsArray) {
                        delete QSProcessedFieldsArray[fieldToCheck];
                    }
                    //Needs to be called to enable quicksearch/typeahead functionality on the field.
                    enableQS(true);
                }

and also I find DOMPurify in suitecrm \vendor\tinymce\tinymce\themes\silver\theme.js

2

Why do you need to sanitize, is it the eval?

Because the content being evaled is coming from the server, it is surely safe.

3

no no,

//If we find all the required variables to do inline editing.
        if (field && id && module) {
            //Do ajax call to retrieve the validation for the field.
            var validation = getValidationRules(field, module, id);
            //Do ajax call to retrieve the html elements of the field.
            var html = loadFieldHTML(field, module, id);

            //If we have the field html append it to the div we clicked.
            if (html) {
                console.log("dsfds" + validation);
                $(_this).html(
                    validation +
                        "<form name='EditView' id='EditView'><div id='inline_edit_field'>" +
                        html +
                        "</div><a id='inlineEditSaveButton'></a></form>"
                );
                $("#inlineEditSaveButton").html(
                    '<span class="suitepicon suitepicon-action-confirm"></span>'
                );
                //If the field is a relate field we will need to retrieve the extra js required to make the field work.
                if (type == "relate" || type == "parent") {
                    var relate_js = getRelateFieldJS(field, module, id);
                    $(_this).append(relate_js);
                    SUGAR.util.evalScript($(_this).html());
                    // Issue 2344 and 2499 changes - Dump existing QSProcessedFieldsArray to enable multiple QS on multiple rows.
                    var fieldToCheck = "EditView_" + field + "_display";
                    if (fieldToCheck in QSProcessedFieldsArray) {
                        delete QSProcessedFieldsArray[fieldToCheck];
                    }
                    //Needs to be called to enable quicksearch/typeahead functionality on the field.
                    enableQS(true);
                }

                //Add the active class so we know which td we are editing as they all have the inlineEdit class.
                $(_this).addClass("inlineEditActive");

                //Put the cursor in the field if possible.
                $("#" + field).focus();
                if (type == "name" || type == "text") {
                    // move focus to end of text (multiply by 2 to make absolute certain its end as some browsers count carriage return as more than 1 character)
                    var strLength = $("#" + field).val().length * 2;
                    $("#" + field)[0].setSelectionRange(strLength, strLength);
                }

                //We can only edit one field at a time currently so turn off the on dblclick event
                $(".inlineEdit").off("click");
                $(".inlineEdit").off("dblclick");

                //Call the click away function to handle if the user has clicked off the field, if they have it will close the form.
                clickedawayclose(field, id, module, type);

                //Make sure the data is valid and save the details to the bean.
                validateFormAndSave(field, id, module, type);
            }
        }

here getValidationRules function is calling which is

/**
 * Ajax call to retrieve the field validation js this needs to be done separately as you can't json_encode javascript.
 * Calls a controller action in /modules/Home/controller.
 * Returns the add to validate call for the field..
 * @param field
 * @param module
 * @param id
 * @returns {*}
 */
function getValidationRules(field, module, id) {
    $.ajaxSetup({ async: false });
    var result = $.getJSON("index.php", {
        module: "Home",
        action: "getValidationRules",
        field: field,
        current_module: module,
        id: id,
        to_pdf: true
    });
    $.ajaxSetup({ async: true });

    try {
        var validation = JSON.parse(result.responseText);
    } catch (e) {
        alert(
            SUGAR.language.translate(
                "app_strings",
                "LBL_LOADING_ERROR_INLINE_EDITING"
            )
        );
        return false;
    }

    return (
        "<script type='text/javascript'>addToValidate('EditView', \"" + field + '", "' + validation["type"] + '", ' + validation["required"] + ',"' + validation["label"] + '");</script>'
    );
}

I want to sanitize include/InlineEditing/inlineEditing.js, line 153 (Cross-Site Scripting: DOM)
Kingdom: Input Validation and Representation
Scan Engine: SCA (Data Flow)
Source Details
Source: Read result.responseText
From: getValidationRules
File: include/InlineEditing/inlineEditing.js

and sinking details is
Sink: ~JS_Generic.html()
Enclosing Method: onInlineEditDblClick()
File: include/InlineEditing/inlineEditing.js
Taint Flags: JS_OBJECT_CONTROLLED, WEB, XSS

4

Ok, but how is that untrusted input?

If you trust getValidationRules responseText, and loadFieldHtml (both are SuiteCRM back-end functions, not dependent on user input), then you can trust that line 153 in inlineEditing.js.

Am I right? If not, please explain how that could be exploited.

6

Hey pgr,

You’re right that getValidationRules and loadFieldHTML are back-end functions in SuiteCRM, and ideally, they should be returning safe, controlled data since they’re server-side. However, the concern here isn’t necessarily about distrusting the back-end logic itself—it’s more about the fact that result.responseText from the AJAX call is being parsed and then directly injected into the DOM via .html() without any sanitization on the client side.

The scan flagged this as a potential XSS (Cross-Site Scripting) vulnerability because if anything goes wrong on the server like a misconfiguration, an exploited endpoint, or even some edge case where user-controlled data sneaks into the response (e.g., through a field label or validation rule) it could allow malicious JavaScript to be executed. For example, if an attacker somehow influenced the validation JSON payload to include something like “label”: test<script>alert('xss')</script>, that script could end up in the DOM since $(this).html() doesn’t filter it out.

7

Your analysis is correct, but in the end this is just a false positive from your scanner. If anything needed to be fixed it would be in the backend functions, not here.

To remove this here would involve a complete overhaul of the validation process in v7, and that is definitely not going to happen. This is probably the final year of life of this 20-year-old bit of code… Let’s just let it die peacefully :slight_smile:

Remember, “sanitization” is short for “sanitization of user input”, or of other untrusted data coming into the system from the outside. There is no such thing happening here. “Yes, but there could be” is not enough to get me worried.

The best thing we can do for SuiteCRM security at this point is simply to get on with the move to v8 code :slight_smile:

8

@Clay You can use DOMPurify to sanitize the html content before injecting it into the DOM. Modify your code like this:

if (html) {
    // Ensure DOMPurify is loaded
    if (typeof DOMPurify !== "undefined") {
        html = DOMPurify.sanitize(html);
    }

    $(_this).html(
        validation +
            "<form name='EditView' id='EditView'><div id='inline_edit_field'>" +
            html +
            "</div><a id='inlineEditSaveButton'></a></form>"
    );
    $("#inlineEditSaveButton").html(
        '<span class="suitepicon suitepicon-action-confirm"></span>'
    );

    if (type == "relate" || type == "parent") {
        var relate_js = getRelateFieldJS(field, module, id);
        $(_this).append(relate_js);
        SUGAR.util.evalScript($(_this).html());

        var fieldToCheck = "EditView_" + field + "_display";
        if (fieldToCheck in QSProcessedFieldsArray) {
            delete QSProcessedFieldsArray[fieldToCheck];
        }
        enableQS(true);
    }
}