ZooKeeper
  1. ZooKeeper
  2. ZOOKEEPER-1305

zookeeper.c:prepend_string func can dereference null ptr

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 3.3.3
    • Fix Version/s: 3.4.1, 3.3.6, 3.5.0
    • Component/s: c client
    • Labels:
    • Environment:

      All

    • Release Note:
      return ZBADARGUMENTS when passed NULL zhandle instead of dereferencing null pointer.

      Description

      All the callers of the function prepend_string make a call to prepend_string before checking that zhandle_t *zh is not null. At the top of prepend_string, zh is dereferenced without checking for a null ptr:

      static char* prepend_string(zhandle_t zh, const char client_path) {
      char *ret_str;
      if (zh->chroot == NULL)
      return (char *) client_path;

      I propose fixing this by adding the check here in prepend_string:

      static char* prepend_string(zhandle_t zh, const char client_path) {
      char *ret_str;
      if (zh==NULL || zh->chroot == NULL)
      return (char *) client_path;

      1. ZOOKEEPER-1305.patch
        1 kB
        Daniel Lescohier
      2. ZOOKEEPER-1305.patch
        1 kB
        Daniel Lescohier

        Issue Links

          Activity

          Hide
          Patrick Hunt added a comment -

          Thanks for the report, please feel free to submit a patch.

          Show
          Patrick Hunt added a comment - Thanks for the report, please feel free to submit a patch.
          Hide
          Daniel Lescohier added a comment -

          I added a test in TestClient.cc testChroot function because the bug was introduced when the 'chroot' capability was added. All the callers of prepend_string checked zh for NULL and returns ZBADARGUMENTS on NULL, but those functions call prepend_string before the zh checked for NULL; so prepend_string also needs to check for NULL.

          Show
          Daniel Lescohier added a comment - I added a test in TestClient.cc testChroot function because the bug was introduced when the 'chroot' capability was added. All the callers of prepend_string checked zh for NULL and returns ZBADARGUMENTS on NULL, but those functions call prepend_string before the zh checked for NULL; so prepend_string also needs to check for NULL.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12504662/ZOOKEEPER-1305.patch
          against trunk revision 1202557.

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

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

          -1 patch. The patch command could not apply the patch.

          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/794//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12504662/ZOOKEEPER-1305.patch against trunk revision 1202557. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 7 new or modified tests. -1 patch. The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/794//console This message is automatically generated.
          Hide
          Daniel Lescohier added a comment -

          Fix patch; filenames indexed from trunk instead of subdir.

          Show
          Daniel Lescohier added a comment - Fix patch; filenames indexed from trunk instead of subdir.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12504766/ZOOKEEPER-1305.patch
          against trunk revision 1202557.

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

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

          +1 javadoc. The javadoc tool did not generate any warning messages.

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

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

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

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

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/795//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/795//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/795//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12504766/ZOOKEEPER-1305.patch against trunk revision 1202557. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 7 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/795//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/795//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/795//console This message is automatically generated.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12504766/ZOOKEEPER-1305.patch
          against trunk revision 1202557.

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

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

          +1 javadoc. The javadoc tool did not generate any warning messages.

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

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

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

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

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/798//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/798//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/798//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12504766/ZOOKEEPER-1305.patch against trunk revision 1202557. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 7 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/798//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/798//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/798//console This message is automatically generated.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12504766/ZOOKEEPER-1305.patch
          against trunk revision 1208979.

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

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

          +1 javadoc. The javadoc tool did not generate any warning messages.

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

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

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

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

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/814//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/814//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/814//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12504766/ZOOKEEPER-1305.patch against trunk revision 1208979. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 7 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/814//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/814//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/814//console This message is automatically generated.
          Hide
          Mahadev konar added a comment -

          I just committed this to both trunk and 3.4. Thanks Daniel!

          Show
          Mahadev konar added a comment - I just committed this to both trunk and 3.4. Thanks Daniel!
          Hide
          Hudson added a comment -

          Integrated in ZooKeeper-trunk #1389 (See https://builds.apache.org/job/ZooKeeper-trunk/1389/)
          ZOOKEEPER-1305. zookeeper.c:prepend_string func can dereference null ptr. (Daniel Lescohier via mahadev)

          mahadev : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1212153
          Files :

          • /zookeeper/trunk/CHANGES.txt
          • /zookeeper/trunk/src/c/src/zookeeper.c
          • /zookeeper/trunk/src/c/tests/TestClient.cc
          Show
          Hudson added a comment - Integrated in ZooKeeper-trunk #1389 (See https://builds.apache.org/job/ZooKeeper-trunk/1389/ ) ZOOKEEPER-1305 . zookeeper.c:prepend_string func can dereference null ptr. (Daniel Lescohier via mahadev) mahadev : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1212153 Files : /zookeeper/trunk/CHANGES.txt /zookeeper/trunk/src/c/src/zookeeper.c /zookeeper/trunk/src/c/tests/TestClient.cc
          Hide
          Henry Robinson added a comment -

          Hey Mahadev -

          Seems like some people are hitting this bug in 3.3 ZOOKEEPER-1461 - did you mean not to commit this to 3.3? If not, I'll go ahead and commit this there.

          Thanks,

          Henry

          Show
          Henry Robinson added a comment - Hey Mahadev - Seems like some people are hitting this bug in 3.3 ZOOKEEPER-1461 - did you mean not to commit this to 3.3? If not, I'll go ahead and commit this there. Thanks, Henry
          Hide
          Michi Mutsuzaki added a comment -

          I'll patch this to 3.3 branch.

          --Michi

          Show
          Michi Mutsuzaki added a comment - I'll patch this to 3.3 branch. --Michi
          Hide
          Michi Mutsuzaki added a comment -

          Committed to 3.3 branch.

          --Michi

          Show
          Michi Mutsuzaki added a comment - Committed to 3.3 branch. --Michi

            People

            • Assignee:
              Daniel Lescohier
              Reporter:
              Daniel Lescohier
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - 0.5h
                0.5h
                Remaining:
                Remaining Estimate - 0.5h
                0.5h
                Logged:
                Time Spent - Not Specified
                Not Specified

                  Development