Improved: Sanitize all widget xml resource loading

[Krishnauprit18]

This PR builds upon PR #1552 by preserving WidgetSecureLocation architecture and introducing two Defense-in-Depth security add-ons in UtilValidate:

1. Protocol-Layer Rejection (file:/): Extends isUrlInStringAndDoesNotStartByComponentProtocol to explicitly reject file:/ schemes at the entry point, preventing single-slash URL bypasses.

2. Default-Deny Policy: Enforces an explicit default-deny check when allowFilePaths in security.properties is unconfigured or blank.

[Copilot]

Pull request overview

This PR introduces a centralized sanitizer (WidgetSecureLocation) intended to harden widget XML resource loading by only allowing component:// locations (without traversal) or explicitly permitted filesystem paths via security.properties.

Changes:

• Added WidgetSecureLocation and integrated it into multiple widget factories before resolving/loading XML resources.
• Added a new default-deny allowFilePaths property and implemented UtilValidate.isAllowedPath(...) to enforce it.
• Updated one screen include to use component://... instead of a relative path.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
framework/widget/src/main/java/org/apache/ofbiz/widget/model/WidgetSecureLocation.java	Adds a centralized location sanitizer for widget resource loading.
framework/widget/src/main/java/org/apache/ofbiz/widget/model/TreeFactory.java	Uses sanitizer before resolving/loading tree XML resources.
framework/widget/src/main/java/org/apache/ofbiz/widget/model/ScreenFactory.java	Uses sanitizer for referenced screen includes.
framework/widget/src/main/java/org/apache/ofbiz/widget/model/MenuFactory.java	Uses sanitizer before resolving/loading menu XML resources.
framework/widget/src/main/java/org/apache/ofbiz/widget/model/GridFactory.java	Uses sanitizer before resolving/loading grid XML resources.
framework/widget/src/main/java/org/apache/ofbiz/widget/model/FormFactory.java	Uses sanitizer before resolving/loading form XML resources.
framework/security/config/security.properties	Introduces allowFilePaths configuration knob (default blank).
framework/base/src/main/java/org/apache/ofbiz/base/util/UtilValidate.java	Adds isAllowedPath and changes URL detection behavior used by loaders/sanitizer.
applications/commonext/widget/ofbizsetup/ProfileScreens.xml	Migrates include-screen locations to component://....

Suppressed comments (4)

framework/widget/src/main/java/org/apache/ofbiz/widget/model/TreeFactory.java:65

• After resolving a sanitized component:// location, treeFileUrl.toString() is usually file:/.... With the current isUrlInStringAndDoesNotStartByComponentProtocol behavior, that check can reject valid local file URLs and abort tree loading. Also, pass the sanitized (normalized) location through to the model to avoid cache/model inconsistencies.

            if (treeFileUrl == null || UtilValidate.isUrlInStringAndDoesNotStartByComponentProtocol(treeFileUrl.toString())) {
                throw new IllegalArgumentException("Could not resolve location to URL: " + resourceName);
            }
            Document treeFileDoc = UtilXml.readXmlDocument(treeFileUrl, true, true);
            modelTreeMap = readTreeDocument(treeFileDoc, delegator, dispatcher, resourceName);

framework/widget/src/main/java/org/apache/ofbiz/widget/model/MenuFactory.java:127

• menuFileUrl.toString() for resolved component:// resources is typically file:/..., so using isUrlInStringAndDoesNotStartByComponentProtocol(...) here can reject valid local URLs. Use isUrlInString(...) (or only validate the original string before resolving) and propagate the sanitized location to downstream model parsing.

            if (menuFileUrl == null || UtilValidate.isUrlInStringAndDoesNotStartByComponentProtocol(menuFileUrl.toString())) {
                throw new IllegalArgumentException("Could not resolve location to URL: " + resourceName);
            }
            Document menuFileDoc = UtilXml.readXmlDocument(menuFileUrl, true, true);
            modelMenuMap = readMenuDocument(menuFileDoc, resourceName, visualTheme);

framework/widget/src/main/java/org/apache/ofbiz/widget/model/GridFactory.java:87

• For sanitized component:// locations, gridFileUrl.toString() is generally file:/.... Using isUrlInStringAndDoesNotStartByComponentProtocol(...) here can therefore reject valid local URLs and abort grid loading. Also prefer using the sanitized/normalized location consistently in error messages and model creation.

            URL gridFileUrl = FlexibleLocation.resolveLocation(sanitizedLocation);
            if (gridFileUrl == null || UtilValidate.isUrlInStringAndDoesNotStartByComponentProtocol(gridFileUrl.toString())) {
                throw new IllegalArgumentException("Could not resolve location to URL: " + resourceName);
            }
            Document gridFileDoc = UtilXml.readXmlDocument(gridFileUrl, true, true);

framework/widget/src/main/java/org/apache/ofbiz/widget/model/FormFactory.java:85

• For sanitized component:// locations, formFileUrl.toString() is generally file:/.... Using isUrlInStringAndDoesNotStartByComponentProtocol(...) can reject valid local URLs and abort form loading. Also use the sanitized/normalized location consistently in messages and model creation to avoid inconsistencies.

            URL formFileUrl = FlexibleLocation.resolveLocation(sanitizedLocation);
            if (formFileUrl == null || UtilValidate.isUrlInStringAndDoesNotStartByComponentProtocol(formFileUrl.toString())) {
                throw new IllegalArgumentException("Could not resolve location to URL: " + resourceName);
            }
            Document formFileDoc = UtilXml.readXmlDocument(formFileUrl, true, true);

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[ashishvijaywargiya]

Hello @Krishnauprit18,

Your changes have been merged using this PR - #1654

Thank you for your kind support.

Please close this PR whenever you can.

Thanks,
Ashish