CVE-2021-21645

@@ -106,7 +106,10 @@ public String getDisplayName() {
             return "";
         }
 
-        public ListBoxModel doFillFileIdItems(@AncestorInPath ItemGroup context) {
+        public ListBoxModel doFillFileIdItems(@AncestorInPath ItemGroup context, @AncestorInPath Item project) {
+            // You should have permission to configure your project in order to get the available managed files
+            project.checkPermission(Item.CONFIGURE);
+            
             ListBoxModel items = new ListBoxModel();
             items.add("please select", "");
             for (Config config : ConfigFiles.getConfigsInContext(context, null)) {
@@ -124,6 +127,10 @@ public ListBoxModel doFillFileIdItems(@AncestorInPath ItemGroup context) {
          * @return a validation result / description
          */
         public HttpResponse doCheckFileId(StaplerRequest req, @AncestorInPath Item context, @QueryParameter String fileId) {
+            // You should have permission to configure your project in order to check whether the selected file id is
+            // allowed to you
+            context.checkPermission(Item.CONFIGURE);
+            
             final Config config = ConfigFiles.getByIdOrNull(context, fileId);
             if (config != null) {
                 return ConfigFileDetailLinkDescription.getDescription(req, context, fileId);

@@ -67,6 +67,16 @@ public ConfigFilesManagement() {
         this.store = GlobalConfigFiles.get();
     }
 
+    /**
+     * The global configuration actions are exclusive of Jenkins administer.
+     * @return The target.
+     */
+    @Override
+    public Object getTarget() {
+        checkPermission(Jenkins.ADMINISTER);
+        return this;
+    }
+    
     /**
      * @see hudson.model.Action#getDisplayName()
      */
@@ -139,7 +149,7 @@ public Collection<Config> getConfigs() {
      */
     @POST
     public HttpResponse doSaveConfig(StaplerRequest req) {
-        checkPermission(Jenkins.ADMINISTER);
+        // permission handled in getTarget
         try {
             JSONObject json = req.getSubmittedForm().getJSONObject("config");
             Config config = req.bindJSON(Config.class, json);
@@ -158,7 +168,7 @@ public HttpResponse doSaveConfig(StaplerRequest req) {
     }
 
     public void doShow(StaplerRequest req, StaplerResponse rsp, @QueryParameter("id") String configId) throws IOException, ServletException {
-        checkPermission(Jenkins.ADMINISTER);
+        // permission handled in getTarget
 
         Config config = store.getById(configId);
         req.setAttribute("contentType", config.getProvider().getContentType());
@@ -176,7 +186,7 @@ public void doShow(StaplerRequest req, StaplerResponse rsp, @QueryParameter("id"
      * @throws ServletException
      */
     public void doEditConfig(StaplerRequest req, StaplerResponse rsp, @QueryParameter("id") String configId) throws IOException, ServletException {
-        checkPermission(Jenkins.ADMINISTER);
+        // permission handled in getTarget
 
         Config config = store.getById(configId);
         req.setAttribute("contentType", config.getProvider().getContentType());
@@ -196,7 +206,7 @@ public void doEditConfig(StaplerRequest req, StaplerResponse rsp, @QueryParamete
      */
     @POST
     public void doAddConfig(StaplerRequest req, StaplerResponse rsp, @QueryParameter("providerId") String providerId, @QueryParameter("configId") String configId) throws IOException, ServletException {
-        checkPermission(Jenkins.ADMINISTER);
+        // permission handled in getTarget
 
         FormValidation error = null;
         if (providerId == null || providerId.isEmpty()) {
@@ -238,7 +248,7 @@ public void doAddConfig(StaplerRequest req, StaplerResponse rsp, @QueryParameter
     }
 
     public void doSelectProvider(StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException {
-        checkPermission(Jenkins.ADMINISTER);
+        // permission handled in getTarget
         req.setAttribute("providers", ConfigProvider.all());
         req.setAttribute("configId", UUID.randomUUID().toString());
         req.getView(this, JELLY_RESOURCES_PATH + "selectprovider.jelly").forward(req, rsp);
@@ -259,14 +269,16 @@ private void checkPermission(Permission permission) {
      */
     @RequirePOST
     public HttpResponse doRemoveConfig(StaplerRequest res, StaplerResponse rsp, @QueryParameter("id") String configId) throws IOException {
-        checkPermission(Jenkins.ADMINISTER);
+        // permission handled in getTarget
 
         store.remove(configId);
 
         return new HttpRedirect("index");
     }
 
     public FormValidation doCheckConfigId(@QueryParameter("configId") String configId) {
+        // permission handled in getTarget
+
         if (configId == null || configId.isEmpty()) {
             return FormValidation.warning(Messages.ConfigFilesManagement_configIdCannotBeEmpty());
         }
@@ -282,11 +294,4 @@ public FormValidation doCheckConfigId(@QueryParameter("configId") String configI
             return FormValidation.warning(Messages.ConfigFilesManagement_configIdAlreadyUsed(config.name, config.id));
         }
     }
-
-
-    @Override
-    public Object getTarget() {
-        checkPermission(Item.EXTENDED_READ);
-        return this;
-    }
 }

@@ -238,6 +238,8 @@ public HttpResponse doRemoveConfig(StaplerRequest res, StaplerResponse rsp, @Que
 
     @Override
     public FormValidation doCheckConfigId(@QueryParameter("configId") String configId) {
+        checkPermission(Job.CONFIGURE);
+        
         if (configId == null || configId.isEmpty()) {
             return FormValidation.warning(Messages.ConfigFilesManagement_configIdCannotBeEmpty());
         }

@@ -2,6 +2,7 @@
 
 import hudson.Extension;
 import hudson.FilePath;
+import hudson.model.Item;
 import hudson.model.ItemGroup;
 import hudson.model.TaskListener;
 import hudson.slaves.WorkspaceList;
@@ -130,7 +131,9 @@ public String getDisplayName() {
             return "provided global settings.xml";
         }
 
-        public ListBoxModel doFillSettingsConfigIdItems(@AncestorInPath ItemGroup context) {
+        public ListBoxModel doFillSettingsConfigIdItems(@AncestorInPath ItemGroup context, @AncestorInPath Item project) {
+            project.checkPermission(Item.CONFIGURE);
+            
             ListBoxModel items = new ListBoxModel();
             items.add("please select", "");
             for (Config config : ConfigFiles.getConfigsInContext(context, GlobalMavenSettingsConfigProvider.class)) {

@@ -6,9 +6,9 @@
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
+import hudson.model.Item;
 import org.apache.commons.lang.StringUtils;
 import org.jenkinsci.lib.configprovider.model.Config;
-import org.jenkinsci.lib.configprovider.model.ConfigFileManager;
 import org.jenkinsci.plugins.configfiles.ConfigFiles;
 import org.jenkinsci.plugins.configfiles.common.CleanTempFilesAction;
 import org.jenkinsci.plugins.configfiles.maven.MavenSettingsConfig;
@@ -133,7 +133,9 @@ public String getDisplayName() {
             return Messages.MvnSettingsProvider_ProvidedSettings();
         }
 
-        public ListBoxModel doFillSettingsConfigIdItems(@AncestorInPath ItemGroup context) {
+        public ListBoxModel doFillSettingsConfigIdItems(@AncestorInPath ItemGroup context, @AncestorInPath Item project) {
+            project.checkPermission(Item.CONFIGURE);
+            
             ListBoxModel items = new ListBoxModel();
             items.add(Messages.MvnSettingsProvider_PleaseSelect(), "");
             for (Config config : ConfigFiles.getConfigsInContext(context, MavenSettingsConfigProvider.class)) {

@@ -3,6 +3,9 @@
 import com.cloudbees.hudson.plugins.folder.Folder;
 import com.gargoylesoftware.htmlunit.html.HtmlAnchor;
 import com.gargoylesoftware.htmlunit.html.HtmlPage;
+import hudson.model.Item;
+import jenkins.model.Jenkins;
+import org.hamcrest.MatcherAssert;
 import org.hamcrest.Matchers;
 import org.jenkinsci.lib.configprovider.ConfigProvider;
 import org.jenkinsci.lib.configprovider.model.Config;
@@ -19,6 +22,7 @@
 import org.jvnet.hudson.test.BuildWatcher;
 import org.jvnet.hudson.test.Issue;
 import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.MockAuthorizationStrategy;
 
 import java.io.IOException;
 import java.util.Collection;
@@ -27,6 +31,7 @@
 import java.util.concurrent.atomic.AtomicReference;
 
 import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.notNullValue;
 import static org.hamcrest.collection.IsEmptyCollection.empty;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotEquals;
@@ -250,6 +255,40 @@ public void xssPreventionInFolder() throws Exception {
         assertThat(store.getConfigs(), empty());
     }
 
+    @Test
+    @Issue("SECURITY-2203")
+    public void folderCheckConfigIdProtected() throws Exception {
+        // ----------
+        // Create a new folder
+        Folder f1 = createFolder();
+        f1.save();
+
+        // ----------
+        // let's allow all people to see the folder, but not configure it
+        r.jenkins.setSecurityRealm(r.createDummySecurityRealm());
+        r.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy()
+                // read access to everyone
+                .grant(Jenkins.READ).everywhere().toEveryone()
+                .grant(Item.DISCOVER).everywhere().toAuthenticated()
+                .grant(Item.READ).onItems(f1).toEveryone()
+                        
+                // config access on the folder to this user
+                .grant(Item.CONFIGURE).onFolders(f1).to("folderConfigurer")
+        );
+
+        // ----------
+        // An user without permission cannot see the form to add a new config file
+        JenkinsRule.WebClient wc = r.createWebClient();
+        wc.login("reader");
+        wc.assertFails(f1.getUrl() +  "configfiles/selectProvider", 404);
+
+        // ----------
+        // The person with permission can access
+        wc.login("folderConfigurer");
+        HtmlPage page = wc.goTo(f1.getUrl() +  "configfiles/selectProvider");
+        MatcherAssert.assertThat(page, notNullValue());
+    }
+    
     private CpsFlowDefinition getNewJobDefinition() {
         return new CpsFlowDefinition("" +
                 "node {\n" +

@@ -0,0 +1,155 @@
+package org.jenkinsci.plugins.configfiles;
+
+import hudson.model.FreeStyleProject;
+import hudson.model.Item;
+import hudson.model.ItemGroup;
+import hudson.model.User;
+import hudson.security.ACL;
+import hudson.security.ACLContext;
+import hudson.security.AccessDeniedException2;
+import hudson.security.Permission;
+import jenkins.model.Jenkins;
+import org.jenkinsci.plugins.configfiles.buildwrapper.ManagedFile;
+import org.jenkinsci.plugins.configfiles.maven.job.MvnGlobalSettingsProvider;
+import org.jenkinsci.plugins.configfiles.maven.job.MvnSettingsProvider;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.jvnet.hudson.test.Issue;
+import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.MockAuthorizationStrategy;
+import org.kohsuke.stapler.StaplerRequest;
+
+import java.io.IOException;
+import java.util.AbstractMap;
+import java.util.Map;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+import static org.junit.Assert.fail;
+
+/**
+ * Testing there is no information disclosure.
+ */
+public class Security2203Test {
+    @Rule
+    public JenkinsRule r = new JenkinsRule();
+
+    private FreeStyleProject project;
+
+    @Before
+    public void setUpAuthorizationAndProject() throws IOException {
+        project = r.jenkins.createProject(FreeStyleProject.class, "j");
+
+        r.jenkins.setSecurityRealm(r.createDummySecurityRealm());
+        r.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy().
+                grant(Jenkins.READ, Item.READ).everywhere().to("reader").
+                grant(Item.CONFIGURE).onItems(project).to("projectConfigurer").
+                grant(Jenkins.ADMINISTER).everywhere().to("administer")
+        );
+    }
+
+    /**
+     * The {@link ManagedFile.DescriptorImpl#doFillFileIdItems(ItemGroup, Item)} is only accessible by people able to
+     * configure the job.
+     */
+    @Issue("SECURITY-2203")
+    @Test
+    public void managedFileDoFillFiledIdItemsProtected() {
+        Runnable run = () -> {
+            ManagedFile.DescriptorImpl descriptor = (ManagedFile.DescriptorImpl) Jenkins.get().getDescriptorOrDie(ManagedFile.class);
+            descriptor.doFillFileIdItems(Jenkins.get(), project);
+        };
+
+        assertWhoCanExecute(run,Item.CONFIGURE, "ManagedFile.DescriptorImpl#doFillFileIdItems");
+    }
+
+    /**
+     * The {@link ManagedFile.DescriptorImpl#doCheckFileId(StaplerRequest, Item, String)} is only accessible by people
+     * able to configure the job.
+     */
+    @Issue("SECURITY-2203")
+    @Test
+    public void managedFileDoCheckFileIdProtected() {
+        Runnable run = () -> {
+            ManagedFile.DescriptorImpl descriptor = (ManagedFile.DescriptorImpl) Jenkins.get().getDescriptorOrDie(ManagedFile.class);
+            descriptor.doCheckFileId(null, project, "fileId"); // request won't be used, we can use null
+        };
+
+        assertWhoCanExecute(run, Item.CONFIGURE, "ManagedFile.DescriptorImpl#doCheckFileId");
+    }
+    
+    /**
+     * The {@link MvnGlobalSettingsProvider.DescriptorImpl#doFillSettingsConfigIdItems(ItemGroup, Item)} is only accessible by people able to
+     * configure the job.
+     */
+    @Issue("SECURITY-2203")
+    @Test
+    public void mvnGlobalSettingsProviderDoFillSettingsConfigIdItemsProtected() {
+        Runnable run = () -> {
+            MvnGlobalSettingsProvider.DescriptorImpl descriptor = (MvnGlobalSettingsProvider.DescriptorImpl) Jenkins.get().getDescriptorOrDie(MvnGlobalSettingsProvider.class);
+            descriptor.doFillSettingsConfigIdItems(Jenkins.get(), project);
+        };
+        
+        assertWhoCanExecute(run, Item.CONFIGURE, "MvnGlobalSettingsProvider.DescriptorImpl#doFillSettingsConfigIdItems");
+    }
+
+    /**
+     * The {@link MvnSettingsProvider.DescriptorImpl#doFillSettingsConfigIdItems(ItemGroup, Item)} is only accessible by people able to
+     * configure the job.
+     */
+    @Issue("SECURITY-2203")
+    @Test
+    public void mvnSettingsProviderDoFillSettingsConfigIdItemsProtected() {
+        Runnable run = () -> {
+            MvnSettingsProvider.DescriptorImpl descriptor = (MvnSettingsProvider.DescriptorImpl) Jenkins.get().getDescriptorOrDie(MvnSettingsProvider.class);
+            descriptor.doFillSettingsConfigIdItems(Jenkins.get(), project);
+        };
+
+        assertWhoCanExecute(run, Item.CONFIGURE, "MvnSettingsProvider.DescriptorImpl#doFillSettingsConfigIdItems");
+    }
+
+    /**
+     * The {@link ConfigFilesManagement#getTarget()} is only accessible by people able to administer jenkins. It guarantees
+     * all methods in the class require {@link Jenkins#ADMINISTER}.
+     */
+    @Issue("SECURITY-2203")
+    @Test
+    public void configFilesManagementAllMethodsProtected() {
+        Runnable run = () -> {
+            ConfigFilesManagement configFilesManagement = Jenkins.get().getExtensionList(ConfigFilesManagement.class).get(0);
+            configFilesManagement.getTarget();
+        };
+
+        assertWhoCanExecute(run, Jenkins.ADMINISTER, "ConfigFilesManagement#getTarget");
+    }
+    
+    /**
+     * Common logic to check a specific method is accessible by people with Configure permission and not by any person 
+     * with just read permission. We don't care about the result. If you don't have permission, the method with fail.
+     * @param run The method to check.
+     * @param checkedMethod The name of the method for logging purposes.
+     */
+    private void assertWhoCanExecute(Runnable run, Permission permission, String checkedMethod) {
+        final Map<Permission, String> userWithPermission = Stream.of(
+                new AbstractMap.SimpleEntry<>(Jenkins.READ, "reader"),
+                new AbstractMap.SimpleEntry<>(Item.CONFIGURE, "projectConfigurer"),
+                new AbstractMap.SimpleEntry<>(Jenkins.ADMINISTER, "administer"))
+                .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
+
+        try (ACLContext ctx = ACL.as(User.getOrCreateByIdOrFullName("reader"))) {
+            run.run(); // The method should fail
+            fail(String.format("%s should be only accessible by people with the permission %s, but it's accessible by a person with %s", checkedMethod, permission, Item.READ));
+        } catch (AccessDeniedException2 e) {
+            assertThat(e.permission, equalTo(permission));
+        }
+
+        try (ACLContext ctx = ACL.as(User.getOrCreateByIdOrFullName(userWithPermission.get(permission)))) {
+            run.run(); // The method doesn't fail
+        } catch (AccessDeniedException2 e) {
+            fail(String.format("%s should be accessible to people with the permission %s but it failed with the exception: %s", checkedMethod, permission, e));
+        }
+    }
+}