jenkinsci / jenkins-test-harness

Unit test framework for Jenkins core and its plugins
https://www.jenkins.io/doc/developer/testing/
MIT License
109 stars 126 forks source link

InjectedTest/`JellyTestSuiteBuilder` improperly determines whether an `escape-by-default` Jelly XML PI exists #784

Open daniel-beck opened 4 months ago

daniel-beck commented 4 months ago

https://github.com/jenkinsci/jenkins-test-harness/blob/0622967429467042ee9e0ddf560716a529617755/src/main/java/org/jvnet/hudson/test/JellyTestSuiteBuilder.java#L105 is more lax than https://github.com/jenkinsci/jelly/blob/cb7966734f339619e5ee8e57b08009acbf5e1d10/src/java/org/apache/commons/jelly/parser/XMLParser.java#L800...L810, resulting in code passing the InjectedTest that doesn't actually affect Jelly behavior. See https://github.com/jenkinsci/android-emulator-plugin/pull/207 for an example.

Since https://www.jenkins.io/blog/2018/10/10/security-updates/ this isn't letting a potential vulnerability slip through, as the only effect is potentially not disabling escape-by-default, but might still make for awkward to detect unexpected escaping behavior.