Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improved: Allow to use GroovyDsl in FlexibleStringExpander (OFBIZ-13133) #839

Merged
merged 13 commits into from
Jan 2, 2025

Conversation

nmalin
Copy link
Contributor

@nmalin nmalin commented Oct 10, 2024

Second improvement on this functionality with increase the security by analyse each script to control the presence of potential code injection.

The regexp to control is a property: security.deniedScriptletsTokens.
If a script match the regexp, OFBiz raise in log an alert with the script and the script hash. The script is disabled and can't run.

If you have a safe script who is matched by the regexp, you can add the hash given by OFBiz on the property: security.allowedScriptletHashes

@nmalin nmalin requested a review from gilPts October 10, 2024 12:17

/**
* Load the list of script exception that we autorise to run despite the security risk
* @return Pattern init by the regExp security.deniedScriptletsTokens
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* @return Pattern init by the regExp security.deniedScriptletsTokens
* @return Pattern init by the regExp security.allowedScriptletHashes

Copy link

@gilPts gilPts force-pushed the OFBIZ-13133-secure-groovy-script branch from 067b047 to e05afe9 Compare November 28, 2024 14:42
Copy link

sonarqubecloud bot commented Dec 2, 2024

nmalin and others added 13 commits January 2, 2025 09:19
Second improvement on this functionality with increase the security by analyse each script to control the presence of potential code injection.

The regexp to control is a property: security.deniedScriptletsTokens.
If a script match the regexp, OFBiz raise in log an alert with the script and the script hash. The script is disabled and can't run.

If you have a safe script who is matched by the regexp, you can add the hash given by OFBiz on the property: security.allowedScriptletHashes
Improve reg exp to support more possible code injection
Improve reg exp to support more possible code injection
Second improvement on this functionality with increase the security by analyse each script to control the presence of potential code injection.

The regexp to control is a property: security.deniedScriptletsTokens.
If a script match the regexp, OFBiz raise in log an alert with the script and the script hash. The script is disabled and can't run.

If you have a safe script who is matched by the regexp, you can add the hash given by OFBiz on the property: security.allowedScriptletHashes
…found.

Test is true if all scriptlet are safe
@nmalin nmalin force-pushed the OFBIZ-13133-secure-groovy-script branch from d84f5a2 to a4b3e35 Compare January 2, 2025 10:45
Copy link

sonarqubecloud bot commented Jan 2, 2025

@nmalin nmalin merged commit 7481ff2 into apache:trunk Jan 2, 2025
7 checks passed
nmalin added a commit that referenced this pull request Jan 2, 2025
…33) (#839)

Second improvement on this functionality with increase the security by analyse each script to control the presence of potential code injection.

The regexp to control is a property: security.deniedScriptletsTokens.
If a script match the regexp, OFBiz raise in log an alert with the script and the script hash. The script is disabled and can't run.

If you have a safe script who is matched by the regexp, you can add the hash given by OFBiz on the property: security.allowedScriptletHashes

We added a new test that scan all xml file to analyse groovy scriplet and return all unsafe scriptlet found. The test will fail if unsafe scriptlets was found.

Thanks to Gil Portenseigne and Jacques Le Roux for help and review
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.

3 participants