fix(setup): refuse server-side pages when installing a theme from the command line - #3473
Merged
Merged
Conversation
… command line The admin screen's theme installer refuses an archive carrying a .jsp, .jspx or .jspf entry (#3466), but `fess-setup install theme` extracted such an archive and installed it. Fess already answers 404 for those extensions under /themes/, so this brings the command-line path in line with the admin screen rather than closing an open hole. - ThemeInstaller checks the extracted files in the staging directory, after the manifest check and before anything installed is moved aside, and fails with a message naming the offending file; the staging directory is removed and fess-setup exits non-zero. - The extension list and the case-insensitive match mirror StaticThemeInstaller. The check is reimplemented in the setup package because fess-setup.jar runs with only the JDK on its class path.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
#3466 made the admin screen's theme installer refuse an archive carrying a
.jsp,.jspxor.jspfentry, butfess-setup install themegoes throughorg.codelibs.fess.setup.ThemeInstaller, which never applied that check: an archive withassets/p.jspwas extracted intoapp/themes/<name>/and the command exited 0.Fess already answers 404 for those extensions under
/themes/(StaticThemeFilter), so this is consistency and defence in depth rather than a fix for an exploitable path.Changes
ThemeInstaller.installchecks the files extracted into the staging directory, after the manifest check and before an installed theme is moved aside. On a match it fails withThe archive has a server-side page, which a static theme may not carry: <path>; the staging directory is removed as for every other failure, the installed theme is left untouched, andfess-setupexits 1.StaticThemeInstaller.isServerSidePage. The check is reimplemented in the setup package rather than shared, becausefess-setup.jarruns with only the JDK on its class path.Testing
ThemeInstallerTest: archives withassets/p.jsp,x.jspx,inc/x.jspfandassets/P.JSPare refused with the entry named in the message and nothing left behind; a refused archive leaves an already-installed theme in place and creates no attic entry;isServerSidePagematching.mvn test -Dtest='org.codelibs.fess.setup.*Test': 205 tests, 0 failures. With the new check disabled the two install tests fail.fess-setup.jarand raninstall theme x:1.0.0 --repository <local repo>against an archive withassets/p.jsp: before, rc=0 and the file was installed; after, rc=1 with the error above and no theme directory created.fess-setup.jarstill reference onlyjava,javax,org.w3c,org.xmlandorg.codelibs.fess.setupclasses.