Uploaded image for project: 'Phoenix'
  1. Phoenix
  2. PHOENIX-4189

Avoid direct use of ObjectInputStream in Hive integration

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 4.12.0
    • Labels:
      None

      Description

      Another security scan ding, but not a very big concern.

      We use ObjectInputStream to serialize/deserialize a Map which contains the columns+values of the primary key constraint. The problem with ObjectInputStream is that it doesn't care what Class it deserializes. If a malicious user can someone coerce some unknowing user to use an InputSplit that has this specially crafted class, we can get into an arbitrary code execution.

      https://www.ibm.com/developerworks/library/se-lookahead/ outlines a way to work around this issue in code, but it leaves a bit to be desired. The ObjectInputStream recursively calls itself as it deserializes the fields in the Object. By trusting some classes from the packages java.lang, java.util, and java.sql, I believe we can remove this minor concern.

      1. PHOENIX-4189_addendum.patch
        1 kB
        James Taylor
      2. PHOENIX-4189.001.patch
        12 kB
        Josh Elser

        Activity

        Hide
        elserj Josh Elser added a comment -

        The static inner-class LookAheadObjectInputStream on PrimaryKeyData is the one that is a little hokey.

        While it makes sense when the PrimaryKeyData object is deserialized, the members are also deserialized through that method: HashMap, String, and all of the values in the Map.

        I can't seem to be able to get the HiveITs running on my laptop (with master). let's see what QA says.

        Show
        elserj Josh Elser added a comment - The static inner-class LookAheadObjectInputStream on PrimaryKeyData is the one that is a little hokey. While it makes sense when the PrimaryKeyData object is deserialized, the members are also deserialized through that method: HashMap , String , and all of the values in the Map . I can't seem to be able to get the HiveITs running on my laptop (with master). let's see what QA says.
        Hide
        elserj Josh Elser added a comment -
        Show
        elserj Josh Elser added a comment - FYI Sergey Soldatov
        Hide
        hadoopqa Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12886224/PHOENIX-4189.001.patch
        against master branch at commit 2ad5d4b48c16743b3f3968a858f9da19c14070fa.
        ATTACHMENT ID: 12886224

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 3 new or modified tests.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        -1 lineLengths. The patch introduces the following lines longer than 100:
        + public static final PrimaryKeyData EMPTY = new PrimaryKeyData(Collections.<String,Object> emptyMap());
        + protected Class<?> resolveClass(ObjectStreamClass desc) throws IOException, ClassNotFoundException {
        + throw new InvalidClassException(desc.getName(), "Expected an instance of PrimaryKeyData");
        + public static PrimaryKeyData deserialize(InputStream input) throws IOException, ClassNotFoundException {
        + throw new InvalidClassException(obj == null ? "null" : obj.getClass().getName(), "Disallowed serialized class");
        + PrimaryKeyData pkCopy = PrimaryKeyData.deserialize(new ByteArrayInputStream(baos.toByteArray()));

        +1 core tests. The patch passed unit tests in .

        Test results: https://builds.apache.org/job/PreCommit-PHOENIX-Build/1415//testReport/
        Console output: https://builds.apache.org/job/PreCommit-PHOENIX-Build/1415//console

        This message is automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12886224/PHOENIX-4189.001.patch against master branch at commit 2ad5d4b48c16743b3f3968a858f9da19c14070fa. ATTACHMENT ID: 12886224 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 3 new or modified tests. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 lineLengths . The patch introduces the following lines longer than 100: + public static final PrimaryKeyData EMPTY = new PrimaryKeyData(Collections.<String,Object> emptyMap()); + protected Class<?> resolveClass(ObjectStreamClass desc) throws IOException, ClassNotFoundException { + throw new InvalidClassException(desc.getName(), "Expected an instance of PrimaryKeyData"); + public static PrimaryKeyData deserialize(InputStream input) throws IOException, ClassNotFoundException { + throw new InvalidClassException(obj == null ? "null" : obj.getClass().getName(), "Disallowed serialized class"); + PrimaryKeyData pkCopy = PrimaryKeyData.deserialize(new ByteArrayInputStream(baos.toByteArray())); +1 core tests . The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-PHOENIX-Build/1415//testReport/ Console output: https://builds.apache.org/job/PreCommit-PHOENIX-Build/1415//console This message is automatically generated.
        Hide
        elserj Josh Elser added a comment -

        Ping Sergey Soldatov

        Looks like the tests passed. Could you take a glance please?

        Show
        elserj Josh Elser added a comment - Ping Sergey Soldatov Looks like the tests passed. Could you take a glance please?
        Hide
        sergey.soldatov Sergey Soldatov added a comment -

        Josh Elser LGTM.

        Show
        sergey.soldatov Sergey Soldatov added a comment - Josh Elser LGTM.
        Hide
        elserj Josh Elser added a comment -

        Pushed, thanks Sergey.

        Show
        elserj Josh Elser added a comment - Pushed, thanks Sergey.
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Jenkins build Phoenix-master #1799 (See https://builds.apache.org/job/Phoenix-master/1799/)
        PHOENIX-4189 Introduce a class that wraps the Map of primary key data (elserj: rev 052490e09f2271eaa84dc9ab123a62a87123a498)

        • (edit) phoenix-hive/src/main/java/org/apache/phoenix/hive/util/PhoenixStorageHandlerUtil.java
        • (add) phoenix-hive/src/main/java/org/apache/phoenix/hive/PrimaryKeyData.java
        • (edit) phoenix-hive/src/main/java/org/apache/phoenix/hive/PhoenixRowKey.java
        • (add) phoenix-hive/src/test/java/org/apache/phoenix/hive/PrimaryKeyDataTest.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Jenkins build Phoenix-master #1799 (See https://builds.apache.org/job/Phoenix-master/1799/ ) PHOENIX-4189 Introduce a class that wraps the Map of primary key data (elserj: rev 052490e09f2271eaa84dc9ab123a62a87123a498) (edit) phoenix-hive/src/main/java/org/apache/phoenix/hive/util/PhoenixStorageHandlerUtil.java (add) phoenix-hive/src/main/java/org/apache/phoenix/hive/PrimaryKeyData.java (edit) phoenix-hive/src/main/java/org/apache/phoenix/hive/PhoenixRowKey.java (add) phoenix-hive/src/test/java/org/apache/phoenix/hive/PrimaryKeyDataTest.java
        Hide
        jamestaylor James Taylor added a comment -

        Addendum patch that removes Eclipse compilation error due to unnecessary cast. FYI, Josh Elser.

        Show
        jamestaylor James Taylor added a comment - Addendum patch that removes Eclipse compilation error due to unnecessary cast. FYI, Josh Elser .
        Hide
        elserj Josh Elser added a comment -

        +1 thanks James. Sorry for missing the warning.

        Show
        elserj Josh Elser added a comment - +1 thanks James. Sorry for missing the warning.
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Jenkins build Phoenix-master #1806 (See https://builds.apache.org/job/Phoenix-master/1806/)
        PHOENIX-4189 Introduce a class that wraps the Map of primary key data (jtaylor: rev e47e78477802940148b6457021a6362cefb002e6)

        • (edit) phoenix-hive/src/main/java/org/apache/phoenix/hive/PrimaryKeyData.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Jenkins build Phoenix-master #1806 (See https://builds.apache.org/job/Phoenix-master/1806/ ) PHOENIX-4189 Introduce a class that wraps the Map of primary key data (jtaylor: rev e47e78477802940148b6457021a6362cefb002e6) (edit) phoenix-hive/src/main/java/org/apache/phoenix/hive/PrimaryKeyData.java

          People

          • Assignee:
            elserj Josh Elser
            Reporter:
            elserj Josh Elser
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development