James Moger
2013-11-16 6f442a4a7a66a0ba6d5189a0430cd5e95fb39ea8
Automatically adjust web.forwardSlash on Tomcat containers

One issue that frequently arises in the discussion group and the bug
tracker is how Tomcat automatically re-encodes %2f as '/' which breaks
url parameters with %2f. After documenting this in half a dozen places
it still comes up. Clearly I haven't done enough.

Gitblit will now act on, instead of just report, an improperly configured
web.forwardSlash character on Tomcat containers. This will make Gitblit
"just work" for more users and will make the world a better place.

Change-Id: I344428804070a2d6082022cf6b80e2a3d83cea84
2 files modified
72 ■■■■■ changed files
src/main/java/com/gitblit/GitBlit.java 6 ●●●●● patch | view | raw | blame | history
src/main/java/com/gitblit/utils/ContainerUtils.java 66 ●●●●● patch | view | raw | blame | history
src/main/java/com/gitblit/GitBlit.java
@@ -3554,8 +3554,6 @@
        configureFanout();
        configureGitDaemon();
        configureCommitCache();
        ContainerUtils.CVE_2007_0450.test();
    }
    protected void configureMailExecutor() {
@@ -3817,6 +3815,10 @@
                FileSettings settings = new FileSettings(localSettings.getAbsolutePath());
                configureContext(settings, base, true);
            }
            // WAR or Express is likely to be running on a Tomcat.
            // Test for the forward-slash/%2F issue and auto-adjust settings.
            ContainerUtils.CVE_2007_0450.test(settings);
        }
        settingsModel = loadSettingModels();
src/main/java/com/gitblit/utils/ContainerUtils.java
@@ -22,7 +22,7 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import com.gitblit.GitBlit;
import com.gitblit.IStoredSettings;
import com.gitblit.Keys;
/**
@@ -41,37 +41,18 @@
     * @see http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2007-0450
     * @author jpyeron
     */
    public static class CVE_2007_0450
    {
    public static class CVE_2007_0450 {
        /**
         * This method will test for know issues in certain containers where %2F
         * is blocked from use in URLs. It will emit a warning to the logger if
         * the configuration of Tomcat causes the URL processing to fail on %2F.
         */
        public static void test()
        {
            if (GitBlit.getBoolean(Keys.web.mountParameters, true)
                    && ((GitBlit.getChar(Keys.web.forwardSlashCharacter, '/')) == '/' || (GitBlit.getChar(
                            Keys.web.forwardSlashCharacter, '/')) == '\\'))
            {
                try
                {
                    if (GitBlit.isGO())
                        ;
                    else if (logCVE_2007_0450Tomcat())
                        ;
                    // else if (logCVE_2007_0450xxx());
                    else
                    {
                        LOGGER.info("Unknown container, cannot check for CVE-2007-0450 aplicability");
                    }
                }
                catch (Throwable t)
                {
                    LOGGER.warn("Failure in checking for CVE-2007-0450 aplicability", t);
                }
        public static void test(IStoredSettings settings) {
            boolean mounted = settings.getBoolean(Keys.web.mountParameters, true);
            char fsc = settings.getChar(Keys.web.forwardSlashCharacter, '/');
            if (mounted && ((fsc == '/') || (fsc == '\\'))) {
                logCVE_2007_0450Tomcat(settings);
            }
        }
        /**
@@ -84,10 +65,8 @@
         * @return true if it recognizes Tomcat, false if it does not recognize
         *         Tomcat
         */
        private static boolean logCVE_2007_0450Tomcat()
        {
            try
            {
        private static boolean logCVE_2007_0450Tomcat(IStoredSettings settings) {
            try {
                byte[] test = "http://server.domain:8080/context/servlet/param%2fparam".getBytes();
                // ByteChunk mb=new ByteChunk();
@@ -95,37 +74,34 @@
                Object mb = cByteChunk.newInstance();
                // mb.setBytes(test, 0, test.length);
                Method mByteChunck_setBytes = cByteChunk.getMethod("setBytes", byte[].class, int.class, int.class);
                mByteChunck_setBytes.invoke(mb, test, 0, test.length);
                Method setBytes = cByteChunk.getMethod("setBytes", byte[].class, int.class, int.class);
                setBytes.invoke(mb, test, 0, test.length);
                // UDecoder ud=new UDecoder();
                Class<?> cUDecoder = Class.forName("org.apache.tomcat.util.buf.UDecoder");
                Object ud = cUDecoder.newInstance();
                // ud.convert(mb,false);
                Method mUDecoder_convert = cUDecoder.getMethod("convert", cByteChunk, boolean.class);
                Method convert = cUDecoder.getMethod("convert", cByteChunk, boolean.class);
                try
                {
                    mUDecoder_convert.invoke(ud, mb, false);
                }
                catch (InvocationTargetException e)
                {
                    if (e.getTargetException() != null && e.getTargetException() instanceof CharConversionException)
                    {
                try {
                    convert.invoke(ud, mb, false);
                } catch (InvocationTargetException e) {
                    if (e.getTargetException() != null && e.getTargetException() instanceof CharConversionException) {
                        LOGGER.warn("You are using a Tomcat-based server and your current settings will prevent grouped repositories, forks, personal repositories, and tree navigation from working properly. Please review the FAQ for details about running Gitblit on Tomcat. http://gitblit.com/faq.html and http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2007-0450");
                        LOGGER.warn("Overriding {} and setting to {}", Keys.web.forwardSlashCharacter, "!");
                        settings.overrideSetting(Keys.web.forwardSlashCharacter, "!");
                        return true;
                    }
                    throw e;
                }
            }
            catch (Throwable t)
            {
            } catch (Throwable t) {
                // The apache url decoder internals are different, this is not a
                // Tomcat matching the failure pattern for CVE-2007-0450
                if (t instanceof ClassNotFoundException || t instanceof NoSuchMethodException
                        || t instanceof IllegalArgumentException)
                        || t instanceof IllegalArgumentException) {
                    return false;
                }
                LOGGER.debug("This is a tomcat, but the test operation failed somehow", t);
            }
            return true;