Improve CodeQL handling of multiple rules (#474)

Also fixed incidental bug in header injection remediation when applied
to interfaces.

core-codemods/src/main/java/io/codemodder/codemods/codeql/CodeQLXSSCodemod.java

@@ -48,8 +48,6 @@ public CodemodFileScanningResult visit(
         r ->
             Optional.ofNullable(
                 r.getLocations().get(0).getPhysicalLocation().getRegion().getEndLine()),
-        r ->
-            Optional.ofNullable(
-                r.getLocations().get(0).getPhysicalLocation().getRegion().getStartColumn()));
+        r -> Optional.empty());
   }
 }

core-codemods/src/test/java/io/codemodder/codemods/codeql/CodeQLHttpResponseSplittingCodemodTest.java

@@ -0,0 +1,15 @@
+package io.codemodder.codemods.codeql;
+
+import io.codemodder.DependencyGAV;
+import io.codemodder.testutils.CodemodTestMixin;
+import io.codemodder.testutils.Metadata;
+
+@Metadata(
+    codemodType = CodeQLHttpResponseSplittingCodemod.class,
+    testResourceDir = "codeql-http-response-splitting",
+    expectingFixesAtLines = {155},
+    doRetransformTest = false,
+    renameTestFile =
+        "app/src/main/java/org/apache/roller/weblogger/webservices/oauth/AuthorizationServlet.java",
+    dependencies = DependencyGAV.JAVA_SECURITY_TOOLKIT_GAV)
+final class CodeQLHttpResponseSplittingCodemodTest implements CodemodTestMixin {}

core-codemods/src/test/java/io/codemodder/codemods/codeql/CodeQLXSSCodemodTest.java

@@ -0,0 +1,14 @@
+package io.codemodder.codemods.codeql;
+
+import io.codemodder.testutils.CodemodTestMixin;
+import io.codemodder.testutils.Metadata;
+
+@Metadata(
+    codemodType = CodeQLXSSCodemod.class,
+    testResourceDir = "codeql-xss",
+    renameTestFile =
+        "app/src/main/java/org/apache/roller/weblogger/ui/core/tags/calendar/CalendarTag.java",
+    expectingFixesAtLines = 302,
+    doRetransformTest = false,
+    dependencies = {})
+final class CodeQLXSSCodemodTest implements CodemodTestMixin {}

core-codemods/src/test/resources/codeql-http-response-splitting/AuthorizationServlet.java.after

@@ -0,0 +1,171 @@
+/*
+ * Copyright 2007 AOL, LLC.
+ * Portions Copyright 2009 Apache Software Foundation
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.roller.weblogger.webservices.oauth;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServlet;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+import net.oauth.OAuth;
+import net.oauth.OAuthAccessor;
+import net.oauth.OAuthMessage;
+import net.oauth.server.OAuthServlet;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.apache.roller.weblogger.business.OAuthManager;
+import org.apache.roller.weblogger.business.WebloggerFactory;
+
+/**
+ * Authorization request handler.
+ *
+ * @author Praveen Alavilli
+ * @author Dave Johnson (adapted for Roller)
+ */
+public class AuthorizationServlet extends HttpServlet {
+    protected static final Log log = LogFactory.getFactory().getInstance(AuthorizationServlet.class);
+
+    @Override
+    public void doGet(HttpServletRequest request, HttpServletResponse response)
+            throws IOException, ServletException {
+
+        try{
+            OAuthMessage requestMessage = OAuthServlet.getMessage(request, null);
+
+            OAuthManager omgr = WebloggerFactory.getWeblogger().getOAuthManager();
+            OAuthAccessor accessor = omgr.getAccessor(requestMessage);
+
+            if (Boolean.TRUE.equals(accessor.getProperty("authorized"))) {
+                // already authorized send the user back
+                returnToConsumer(request, response, accessor);
+            } else {
+                sendToAuthorizePage(request, response, accessor);
+            }
+
+        } catch (Exception e){
+            handleException(e, request, response, true);
+        }
+    }
+
+    @Override
+    public void doPost(HttpServletRequest request, HttpServletResponse response)
+            throws IOException, ServletException {
+
+        try{
+            OAuthMessage requestMessage = OAuthServlet.getMessage(request, null);
+
+            OAuthManager omgr = WebloggerFactory.getWeblogger().getOAuthManager();
+            OAuthAccessor accessor = omgr.getAccessor(requestMessage);
+
+            String userId = request.getParameter("userId");
+            if (userId == null) {
+                userId = request.getParameter("xoauth_requestor_id");
+            }
+
+            if (userId == null) {
+                // no user associted with the key, must be site-wide key,
+                // so get user to login and do the authorization process
+                sendToAuthorizePage(request, response, accessor);
+
+            } else {
+
+                // if consumer key is for specific user, check username match
+                String consumerUserId = (String)accessor.consumer.getProperty("userId");
+                if (consumerUserId != null && !userId.equals(consumerUserId)) {
+                    throw new ServletException("ERROR: invalid or unspecified userId");
+                }
+
+                // set userId in accessor and mark it as authorized
+                omgr.markAsAuthorized(accessor, userId);
+                WebloggerFactory.getWeblogger().flush();
+            }
+
+            returnToConsumer(request, response, accessor);
+
+        } catch (Exception e){
+            handleException(e, request, response, true);
+        }
+    }
+
+    private void sendToAuthorizePage(HttpServletRequest request,
+            HttpServletResponse response, OAuthAccessor accessor)
+    throws IOException, ServletException{
+        String callback = request.getParameter("oauth_callback");
+        if(callback == null || callback.length() <=0) {
+            callback = "none";
+        }
+        String consumer_description = (String)accessor.consumer.getProperty("description");
+        request.setAttribute("CONS_DESC", consumer_description);
+        request.setAttribute("CALLBACK", callback);
+        request.setAttribute("TOKEN", accessor.requestToken);
+        request.getRequestDispatcher("/roller-ui/oauthAuthorize.rol").forward(request, response);
+    }
+
+    private void returnToConsumer(HttpServletRequest request,
+            HttpServletResponse response, OAuthAccessor accessor)
+        throws IOException, ServletException {
+
+        // send the user back to site's callBackUrl
+        String callback = request.getParameter("oauth_callback");
+        if ("none".equals(callback)
+            && accessor.consumer.callbackURL != null
+                && accessor.consumer.callbackURL.length() > 0){
+            // first check if we have something in our properties file
+            callback = accessor.consumer.callbackURL;
+        }
+
+        if ( "none".equals(callback) ) {
+            // no call back it must be a client
+            response.setContentType("text/plain");
+            try (PrintWriter out = response.getWriter()) {
+                out.println("You have successfully authorized for consumer key '"
+                        + accessor.consumer.consumerKey
+                        + "'. Please close this browser window and click continue"
+                                + " in the client.");
+            }
+        } else {
+            // if callback is not passed in, use the callback from config
+            if(callback == null || callback.length() <=0 ) {
+                callback = accessor.consumer.callbackURL;
+            }
+            String token = accessor.requestToken;
+            if (token != null && callback != null) {
+                callback = OAuth.addParameters(callback, "oauth_token", token);
+            }
+
+            response.setStatus(HttpServletResponse.SC_MOVED_TEMPORARILY);
+            response.setHeader("Location", stripNewlines(callback));
+        }
+    }
+
+    public void handleException(Exception e, HttpServletRequest request,
+            HttpServletResponse response, boolean sendBody)
+            throws IOException, ServletException {
+        log.debug("ERROR authorizing token", e);
+        String realm = (request.isSecure())?"https://":"http://";
+        realm += request.getLocalName();
+        OAuthServlet.handleException(response, e, realm, sendBody);
+    }
+
+    private static String stripNewlines(final String s) {
+        return s.replaceAll("[\n\r]", "");
+    }
+}

core-codemods/src/test/resources/codeql-http-response-splitting/AuthorizationServlet.java.before

@@ -0,0 +1,167 @@
+/*
+ * Copyright 2007 AOL, LLC.
+ * Portions Copyright 2009 Apache Software Foundation
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.roller.weblogger.webservices.oauth;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServlet;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+import net.oauth.OAuth;
+import net.oauth.OAuthAccessor;
+import net.oauth.OAuthMessage;
+import net.oauth.server.OAuthServlet;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.apache.roller.weblogger.business.OAuthManager;
+import org.apache.roller.weblogger.business.WebloggerFactory;
+
+/**
+ * Authorization request handler.
+ *
+ * @author Praveen Alavilli
+ * @author Dave Johnson (adapted for Roller)
+ */
+public class AuthorizationServlet extends HttpServlet {
+    protected static final Log log = LogFactory.getFactory().getInstance(AuthorizationServlet.class);
+
+    @Override
+    public void doGet(HttpServletRequest request, HttpServletResponse response)
+            throws IOException, ServletException {
+
+        try{
+            OAuthMessage requestMessage = OAuthServlet.getMessage(request, null);
+
+            OAuthManager omgr = WebloggerFactory.getWeblogger().getOAuthManager();
+            OAuthAccessor accessor = omgr.getAccessor(requestMessage);
+
+            if (Boolean.TRUE.equals(accessor.getProperty("authorized"))) {
+                // already authorized send the user back
+                returnToConsumer(request, response, accessor);
+            } else {
+                sendToAuthorizePage(request, response, accessor);
+            }
+
+        } catch (Exception e){
+            handleException(e, request, response, true);
+        }
+    }
+
+    @Override
+    public void doPost(HttpServletRequest request, HttpServletResponse response)
+            throws IOException, ServletException {
+
+        try{
+            OAuthMessage requestMessage = OAuthServlet.getMessage(request, null);
+
+            OAuthManager omgr = WebloggerFactory.getWeblogger().getOAuthManager();
+            OAuthAccessor accessor = omgr.getAccessor(requestMessage);
+
+            String userId = request.getParameter("userId");
+            if (userId == null) {
+                userId = request.getParameter("xoauth_requestor_id");
+            }
+
+            if (userId == null) {
+                // no user associted with the key, must be site-wide key,
+                // so get user to login and do the authorization process
+                sendToAuthorizePage(request, response, accessor);
+
+            } else {
+
+                // if consumer key is for specific user, check username match
+                String consumerUserId = (String)accessor.consumer.getProperty("userId");
+                if (consumerUserId != null && !userId.equals(consumerUserId)) {
+                    throw new ServletException("ERROR: invalid or unspecified userId");
+                }
+
+                // set userId in accessor and mark it as authorized
+                omgr.markAsAuthorized(accessor, userId);
+                WebloggerFactory.getWeblogger().flush();
+            }
+
+            returnToConsumer(request, response, accessor);
+
+        } catch (Exception e){
+            handleException(e, request, response, true);
+        }
+    }
+
+    private void sendToAuthorizePage(HttpServletRequest request,
+            HttpServletResponse response, OAuthAccessor accessor)
+    throws IOException, ServletException{
+        String callback = request.getParameter("oauth_callback");
+        if(callback == null || callback.length() <=0) {
+            callback = "none";
+        }
+        String consumer_description = (String)accessor.consumer.getProperty("description");
+        request.setAttribute("CONS_DESC", consumer_description);
+        request.setAttribute("CALLBACK", callback);
+        request.setAttribute("TOKEN", accessor.requestToken);
+        request.getRequestDispatcher("/roller-ui/oauthAuthorize.rol").forward(request, response);
+    }
+
+    private void returnToConsumer(HttpServletRequest request,
+            HttpServletResponse response, OAuthAccessor accessor)
+        throws IOException, ServletException {
+
+        // send the user back to site's callBackUrl
+        String callback = request.getParameter("oauth_callback");
+        if ("none".equals(callback)
+            && accessor.consumer.callbackURL != null
+                && accessor.consumer.callbackURL.length() > 0){
+            // first check if we have something in our properties file
+            callback = accessor.consumer.callbackURL;
+        }
+
+        if ( "none".equals(callback) ) {
+            // no call back it must be a client
+            response.setContentType("text/plain");
+            try (PrintWriter out = response.getWriter()) {
+                out.println("You have successfully authorized for consumer key '"
+                        + accessor.consumer.consumerKey
+                        + "'. Please close this browser window and click continue"
+                                + " in the client.");
+            }
+        } else {
+            // if callback is not passed in, use the callback from config
+            if(callback == null || callback.length() <=0 ) {
+                callback = accessor.consumer.callbackURL;
+            }
+            String token = accessor.requestToken;
+            if (token != null && callback != null) {
+                callback = OAuth.addParameters(callback, "oauth_token", token);
+            }
+
+            response.setStatus(HttpServletResponse.SC_MOVED_TEMPORARILY);
+            response.setHeader("Location", callback);
+        }
+    }
+
+    public void handleException(Exception e, HttpServletRequest request,
+            HttpServletResponse response, boolean sendBody)
+            throws IOException, ServletException {
+        log.debug("ERROR authorizing token", e);
+        String realm = (request.isSecure())?"https://":"http://";
+        realm += request.getLocalName();
+        OAuthServlet.handleException(response, e, realm, sendBody);
+    }
+}