checkstyle security issue - nonstatic nested classes and use of Random()

cas-server-core/src/main/java/org/jasig/cas/monitor/AbstractPoolMonitor.java

@@ -18,14 +18,13 @@
  */
 package org.jasig.cas.monitor;
 
+import javax.validation.constraints.NotNull;
 import java.util.concurrent.Callable;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Future;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
 
-import javax.validation.constraints.NotNull;
-
 /**
  * Describes a monitor that observes a pool of resources.
  *
@@ -71,7 +70,7 @@ public void setMaxWait(final int time) {
      **/
     @Override
     public PoolStatus observe() {
-        final Future<StatusCode> result = this.executor.submit(new Validator());
+        final Future<StatusCode> result = this.executor.submit(new Validator(this));
         StatusCode code;
         String description = null;
         try {
@@ -116,11 +115,21 @@ public PoolStatus observe() {
      */
     protected abstract int getActiveCount();
 
+    private static class Validator implements Callable<StatusCode> {
+        private final AbstractPoolMonitor monitor;
+
+        /**
+         * Instantiates a new Validator.
+         *
+         * @param monitor the monitor
+         */
+        public Validator(final AbstractPoolMonitor monitor) {
+            this.monitor = monitor;
+        }
 
-    private class Validator implements Callable<StatusCode> {
         @Override
         public StatusCode call() throws Exception {
-            return checkPool();
+            return this.monitor.checkPool();
         }
     }
 }

cas-server-core/src/main/java/org/jasig/cas/services/AbstractRegisteredService.java

@@ -276,7 +276,7 @@ public final void setLogoutType(final LogoutType logoutType) {
     }
 
     @Override
-    public RegisteredService clone() {
+    public final RegisteredService clone() {
         final AbstractRegisteredService clone = newInstance();
         clone.copyFrom(this);
         return clone;

cas-server-support-jdbc/src/test/java/org/jasig/cas/adaptors/jdbc/QueryAndEncodeDatabaseAuthenticationHandlerTests.java

@@ -192,7 +192,7 @@ private String genPassword(final String psw, final String salt, final int iter)
         }
     }
     @Entity(name="users")
-    public class UsersTable {
+    public static class UsersTable {
         @Id
         @GeneratedValue(strategy = GenerationType.AUTO)
         private Long id;

cas-server-support-jdbc/src/test/java/org/jasig/cas/adaptors/jdbc/QueryDatabaseAuthenticationHandlerTests.java

@@ -88,7 +88,7 @@ private String getSqlInsertStatementToCreateUserAccount(final int i) {
     }
 
     @Entity(name="casusers")
-    public class UsersTable {
+    public static class UsersTable {
         @Id
         @GeneratedValue(strategy = GenerationType.AUTO)
         private Long id;

cas-server-support-jdbc/src/test/java/org/jasig/cas/adaptors/jdbc/SearchModeSearchDatabaseAuthenticationHandlerTests.java

@@ -94,7 +94,7 @@ private String getSqlInsertStatementToCreateUserAccount(final int i) {
     }
 
     @Entity(name="cassearchusers")
-    public class UsersTable {
+    public static class UsersTable {
         @Id
         @GeneratedValue(strategy = GenerationType.AUTO)
         private Long id;

cas-server-support-jdbc/src/test/java/org/jasig/cas/ticket/registry/JpaTicketRegistryTests.java

@@ -133,7 +133,7 @@ public void verifyConcurrentServiceTicketGeneration() throws Exception {
         try {
             final List<ServiceTicketGenerator> generators = new ArrayList<ServiceTicketGenerator>(CONCURRENT_SIZE);
             for (int i = 0; i < CONCURRENT_SIZE; i++) {
-                generators.add(new ServiceTicketGenerator(newTgt.getId()));
+                generators.add(new ServiceTicketGenerator(newTgt.getId(), this.jpaTicketRegistry, this.txManager));
             }
             final List<Future<String>> results = executor.invokeAll(generators);
             for (Future<String> result : results) {
@@ -201,12 +201,16 @@ public ServiceTicket doInTransaction(final TransactionStatus status) {
         });
     }
 
-    class ServiceTicketGenerator implements Callable<String> {
-
+    private static class ServiceTicketGenerator implements Callable<String> {
+        private PlatformTransactionManager txManager;
         private String parentTgtId;
+        private final JpaTicketRegistry jpaTicketRegistry;
 
-        public ServiceTicketGenerator(final String tgtId) {
+        public ServiceTicketGenerator(final String tgtId, final JpaTicketRegistry jpaTicketRegistry,
+                                      final PlatformTransactionManager txManager) {
             parentTgtId = tgtId;
+            this.jpaTicketRegistry = jpaTicketRegistry;
+            this.txManager = txManager;
         }
 
         /**

cas-server-support-jdbc/src/test/java/org/jasig/cas/ticket/registry/support/JpaLockingStrategyTests.java

@@ -18,6 +18,7 @@
  */
 package org.jasig.cas.ticket.registry.support;
 
+import org.jasig.cas.ticket.registry.JpaTicketRegistry;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.slf4j.Logger;
@@ -213,7 +214,7 @@ private LockingStrategy newLockTxProxy(final String appId, final String uniqueId
         return (LockingStrategy) Proxy.newProxyInstance(
                JpaLockingStrategy.class.getClassLoader(),
                new Class[] {LockingStrategy.class},
-               new TransactionalLockInvocationHandler(lock));
+               new TransactionalLockInvocationHandler(lock, this.txManager));
     }
 
     private String getOwner(final String appId) {
@@ -252,11 +253,15 @@ private void testConcurrency(final ExecutorService executor, final LockingStrate
         assertTrue("Release count should be <= 1 but was " + releaseCount, releaseCount <= 1);
     }
 
-    class TransactionalLockInvocationHandler implements InvocationHandler {
-        private JpaLockingStrategy jpaLock;
+    private static class TransactionalLockInvocationHandler implements InvocationHandler {
+        private final Logger logger = LoggerFactory.getLogger(this.getClass());
+        private final JpaLockingStrategy jpaLock;
+        private final PlatformTransactionManager txManager;
 
-        public TransactionalLockInvocationHandler(final JpaLockingStrategy lock) {
+        public TransactionalLockInvocationHandler(final JpaLockingStrategy lock,
+                                      final PlatformTransactionManager txManager) {
             jpaLock = lock;
+            this.txManager = txManager;
         }
 
         public JpaLockingStrategy getLock() {
@@ -285,9 +290,9 @@ public Object doInTransaction(final TransactionStatus status) {
 
     }
 
-    class Locker implements Callable<Boolean> {
-
-        private LockingStrategy lock;
+    private static class Locker implements Callable<Boolean> {
+        private final Logger logger = LoggerFactory.getLogger(this.getClass());
+        private final LockingStrategy lock;
 
         public Locker(final LockingStrategy l) {
             lock = l;
@@ -307,10 +312,9 @@ public Boolean call() throws Exception {
         }
     }
 
-
-    class Releaser implements Callable<Boolean> {
-
-        private LockingStrategy lock;
+    private static class Releaser implements Callable<Boolean> {
+        private final Logger logger = LoggerFactory.getLogger(this.getClass());
+        private final LockingStrategy lock;
 
         public Releaser(final LockingStrategy l) {
             lock = l;

checkstyle-rules.xml

@@ -345,6 +345,12 @@ Checkstyle configuration based on Sun's conventions, compliant with CAS coding c
     <property name="format" value="\(\w+\)\w+"/>
     <property name="message" value="There are no spaces after cast."/>
   </module>
+  <module name="RegexpSingleline">
+    <metadata name="net.sf.eclipsecs.core.comment" value="Non-static inner class"/>
+    <property name="severity" value="error"/>
+    <property name="format" value="\s+(private|public)*\s+(abstract\s)*class\s+\w+"/>
+    <property name="message" value="Non-static inner classes are a security compromise. Consider statics instead"/>
+  </module>
   <module name="RegexpSingleline">
     <metadata name="net.sf.eclipsecs.core.comment" value="Missing @since tag"/>
     <property name="severity" value="error"/>