sqoop-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jar...@apache.org
Subject sqoop git commit: SQOOP-2514: Sqoop2: Findbugs: Fix security problems in ConfigUtils
Date Mon, 24 Aug 2015 16:44:15 GMT
Repository: sqoop
Updated Branches:
  refs/heads/sqoop2 3ee361d8b -> b7ddddbe1


SQOOP-2514: Sqoop2: Findbugs: Fix security problems in ConfigUtils

(Colin Ma via Jarek Jarcec Cecho)


Project: http://git-wip-us.apache.org/repos/asf/sqoop/repo
Commit: http://git-wip-us.apache.org/repos/asf/sqoop/commit/b7ddddbe
Tree: http://git-wip-us.apache.org/repos/asf/sqoop/tree/b7ddddbe
Diff: http://git-wip-us.apache.org/repos/asf/sqoop/diff/b7ddddbe

Branch: refs/heads/sqoop2
Commit: b7ddddbe11f6a544d3a5a8f5579e04fcb0beb4f9
Parents: 3ee361d
Author: Jarek Jarcec Cecho <jarcec@apache.org>
Authored: Mon Aug 24 09:43:52 2015 -0700
Committer: Jarek Jarcec Cecho <jarcec@apache.org>
Committed: Mon Aug 24 09:43:52 2015 -0700

----------------------------------------------------------------------
 .../org/apache/sqoop/model/ConfigUtils.java     | 39 ++++++++++++++------
 1 file changed, 28 insertions(+), 11 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/sqoop/blob/b7ddddbe/common/src/main/java/org/apache/sqoop/model/ConfigUtils.java
----------------------------------------------------------------------
diff --git a/common/src/main/java/org/apache/sqoop/model/ConfigUtils.java b/common/src/main/java/org/apache/sqoop/model/ConfigUtils.java
index 52e7ef6..b4146a7 100644
--- a/common/src/main/java/org/apache/sqoop/model/ConfigUtils.java
+++ b/common/src/main/java/org/apache/sqoop/model/ConfigUtils.java
@@ -30,6 +30,8 @@ import org.apache.sqoop.validation.ConfigValidationResult;
 import org.json.simple.JSONObject;
 
 import java.lang.reflect.Field;
+import java.security.AccessController;
+import java.security.PrivilegedAction;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -83,7 +85,7 @@ public class  ConfigUtils {
 
     // Iterate over all declared fields
     for (Field field : klass.getDeclaredFields()) {
-      field.setAccessible(true);
+      setFieldAccessibleWithAC(field);
 
       // Each field that should be part of user input should have Input
       // annotation.
@@ -127,7 +129,7 @@ public class  ConfigUtils {
 
     // Iterate over all declared fields
     for (Field field : klass.getDeclaredFields()) {
-      field.setAccessible(true);
+      setFieldAccessibleWithAC(field);
 
       String fieldName = field.getName();
       String inputName = configName + "." + fieldName;
@@ -272,7 +274,8 @@ public class  ConfigUtils {
     for(MConfig config : configs) {
       Field configField = getFieldFromName(klass, config.getName());
       // We need to access this field even if it would be declared as private
-      configField.setAccessible(true);
+      setFieldAccessibleWithAC(configField);
+
       Class<?> configClass = configField.getType();
       Object newValue = ClassUtils.instantiate(configClass);
 
@@ -299,7 +302,7 @@ public class  ConfigUtils {
         }
 
         // We need to access this field even if it would be declared as private
-        inputField.setAccessible(true);
+        setFieldAccessibleWithAC(inputField);
 
         try {
           if (input.isEmpty()) {
@@ -386,7 +389,7 @@ public class  ConfigUtils {
 
     // Iterate over all declared fields
     for (Field configField : klass.getDeclaredFields()) {
-      configField.setAccessible(true);
+      setFieldAccessibleWithAC(configField);
 
       // We're processing only config validations
       Config configAnnotation = configField.getAnnotation(Config.class);
@@ -407,7 +410,7 @@ public class  ConfigUtils {
 
       // Now process each input on the config
       for(Field inputField : configField.getType().getDeclaredFields()) {
-        inputField.setAccessible(true);
+        setFieldAccessibleWithAC(inputField);
         String inputName = inputField.getName();
 
         Object value;
@@ -472,7 +475,7 @@ public class  ConfigUtils {
     JSONObject jsonConfigs = JSONUtils.parse(json);
 
     for(Field configField : klass.getDeclaredFields()) {
-      configField.setAccessible(true);
+      setFieldAccessibleWithAC(configField);
 
       // We're processing only config validations
       Config configAnnotation = configField.getAnnotation(Config.class);
@@ -502,7 +505,7 @@ public class  ConfigUtils {
       }
 
       for(Field inputField : configField.getType().getDeclaredFields()) {
-        inputField.setAccessible(true);
+        setFieldAccessibleWithAC(inputField);
         String inputName = inputField.getName();
 
         Input inputAnnotation = inputField.getAnnotation(Input.class);
@@ -525,9 +528,12 @@ public class  ConfigUtils {
           } else if (type.isAssignableFrom(Map.class)) {
             Map<String, String> map = new HashMap<String, String>();
             JSONObject jsonObject = (JSONObject) jsonInputs.get(inputName);
-            for(Object key : jsonObject.keySet()) {
-              map.put((String)key, (String)jsonObject.get(key));
+
+            Set<Map.Entry<String, Object>> entrySet = jsonObject.entrySet();
+            for (Map.Entry<String, Object> entry : entrySet) {
+              map.put(entry.getKey(), (String)entry.getValue());
             }
+
             inputField.set(configValue, map);
           } else if(type == Integer.class) {
             inputField.set(configValue, ((Long)jsonInputs.get(inputName)).intValue());
@@ -635,7 +641,7 @@ public class  ConfigUtils {
 
   public static Object getFieldValue(Field field, Object object) {
     try {
-      field.setAccessible(true);
+      setFieldAccessibleWithAC(field);
       return field.get(object);
     } catch (IllegalAccessException e) {
       throw new SqoopException(ModelError.MODEL_015, e);
@@ -679,4 +685,15 @@ public class  ConfigUtils {
     }
     return;
   }
+
+  // The method setAccessible() requires a security permission check according to the FindBugs's
rule (DP_DO_INSIDE_DO_PRIVILEGED)
+  private static void setFieldAccessibleWithAC(final Field field) {
+    AccessController.doPrivileged(new PrivilegedAction() {
+      @Override
+      public Object run() {
+        field.setAccessible(true);
+        return null;
+      }
+    });
+  }
 }
\ No newline at end of file


Mime
View raw message