Merge 'v2.2.2.2'

* v2.2.2.2:
  Release notes for 2.2.2.2
  Fix permissions bug caused by directly inheriting from All-Projects

Change-Id: Id0a3c91ae300cb7ae177d45578021701ec189f2a

5 files changed, 53 insertions, 4 deletions

diff --git a/ReleaseNotes/ReleaseNotes-2.2.2.2.txt b/ReleaseNotes/ReleaseNotes-2.2.2.2.txt
new file mode 100644
index 0000000000..db5d7505a6
--- /dev/null
+++ b/ReleaseNotes/ReleaseNotes-2.2.2.2.txt
@@ -0,0 +1,24 @@
+Release notes for Gerrit 2.2.2.2
+================================
+
+Gerrit 2.2.2.2 is now available:
+
+link:http://code.google.com/p/gerrit/downloads/detail?name=gerrit-2.2.2.2.war[http://code.google.com/p/gerrit/downloads/detail?name=gerrit-2.2.2.2.war]
+
+There are no schema changes from 2.2.2, or 2.2.2.1.
+
+However, if upgrading from anything earlier, follow the upgrade
+procedure in the 2.2.2 link:ReleaseNotes-2.2.2.html[ReleaseNotes].
+
+Security Fixes
+--------------
+* Some access control sections may be ignored
++
+Gerrit sometimes ignored an access control section in a project
+if the exact same section name appeared in All-Projects. The bug
+required an unrelated project to have access.inheritFrom set to
+All-Projects and be accessed before the project that has the same
+section name as All-Projects. This is an unlikely scenario for
+most servers, as Gerrit does not normally set inheritFrom equal to
+All-Projects. The usual behavior is to not supply this property in
+project.config, and permit the implicit inheritence to take place.
diff --git a/ReleaseNotes/index.txt b/ReleaseNotes/index.txt
index 13da60aa41..51d0b222af 100644
--- a/ReleaseNotes/index.txt
+++ b/ReleaseNotes/index.txt
@@ -10,6 +10,7 @@ Version 2.3.x
 Version 2.2.x
 -------------
 * link:ReleaseNotes-2.2.2.html[2.2.2],
+* link:ReleaseNotes-2.2.2.2.html[2.2.2.2],
 * link:ReleaseNotes-2.2.2.1.html[2.2.2.1],
 * link:ReleaseNotes-2.2.1.html[2.2.1],
 * link:ReleaseNotes-2.2.0.html[2.2.0]
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectState.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectState.java
index 9f6f6e71a8..a72fc1618d 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectState.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectState.java
@@ -197,6 +197,7 @@ public class ProjectState {
 
     List<SectionMatcher> all = new ArrayList<SectionMatcher>();
     Set<Project.NameKey> seen = new HashSet<Project.NameKey>();
+    ProjectState allProjects = projectCache.getAllProjects();
     seen.add(getProject().getNameKey());
 
     ProjectState s = this;
@@ -209,7 +210,9 @@ public class ProjectState {
       }
       s = projectCache.get(parent);
     } while (s != null);
-    all.addAll(projectCache.getAllProjects().getLocalAccessSections());
+    if (seen.add(allProjects.getProject().getNameKey())) {
+      all.addAll(allProjects.getLocalAccessSections());
+    }
     return all;
   }
 
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/SectionSortCache.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/SectionSortCache.java
index c0d2034cf6..2bc957ebcf 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/SectionSortCache.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/SectionSortCache.java
@@ -28,6 +28,8 @@ import com.google.inject.TypeLiteral;
 import com.google.inject.name.Named;
 
 import org.apache.commons.lang.StringUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import java.util.Arrays;
 import java.util.Collections;
@@ -38,6 +40,9 @@ import java.util.List;
 /** Caches the order AccessSections should be sorted for evaluation. */
 @Singleton
 public class SectionSortCache {
+  private static final Logger log =
+      LoggerFactory.getLogger(SectionSortCache.class);
+
   private static final String CACHE_NAME = "permission_sort";
 
   public static Module module() {
@@ -79,10 +84,11 @@ public class SectionSortCache {
       }
 
     } else {
+      boolean poison = false;
       IdentityHashMap<AccessSection, Integer> srcMap =
           new IdentityHashMap<AccessSection, Integer>();
       for (int i = 0; i < cnt; i++) {
-        srcMap.put(sections.get(i), i);
+        poison |= srcMap.put(sections.get(i), i) != null;
       }
 
       Collections.sort(sections, new MostSpecificComparator(ref));
@@ -97,7 +103,11 @@ public class SectionSortCache {
         }
       }
 
-      cache.put(key, new EntryVal(srcIdx));
+      if (poison) {
+        log.error("Received duplicate AccessSection instances, not caching sort");
+      } else {
+        cache.put(key, new EntryVal(srcIdx));
+      }
     }
   }
 
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java
index 29b27628c9..77775d8d15 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java
@@ -145,6 +145,18 @@ public class RefControlTest extends TestCase {
         u.controlForRef("refs/heads/foobar").canUpload());
   }
 
+  public void testInheritDuplicateSections() {
+    grant(parent, READ, admin, "refs/*");
+    grant(local, READ, devs, "refs/heads/*");
+    local.getProject().setParentName(parent.getProject().getName());
+    assertTrue("a can read", user("a", admin).isVisible());
+
+    local = new ProjectConfig(new Project.NameKey("local"));
+    local.createInMemory();
+    grant(local, READ, devs, "refs/*");
+    assertTrue("d can read", user("d", devs).isVisible());
+  }
+
   public void testInheritRead_OverrideWithDeny() {
     grant(parent, READ, registered, "refs/*");
     grant(local, READ, registered, "refs/*").setDeny();
@@ -321,7 +333,6 @@ public class RefControlTest extends TestCase {
 
     local = new ProjectConfig(new Project.NameKey("local"));
     local.createInMemory();
-    local.getProject().setParentName(parent.getProject().getName());
 
     sectionSorter =
         new PermissionCollection.Factory(