Agreed with Enrico and Patrick, those informations in log seems not critical to me, doesn't sound like a "Security issue".
Regards, Fangmin On Mon, Aug 5, 2019 at 10:53 PM Patrick Hunt <[email protected]> wrote: > On Mon, Aug 5, 2019 at 10:17 PM Enrico Olivelli <[email protected]> > wrote: > > > Xiaoqin > > > > Il giorno mar 6 ago 2019 alle ore 03:25 Xiaoqin Fu <[email protected] > > > > ha scritto: > > > > > Dear Patrick Hunt: > > > I opened two issues in the JIRA: > > > https://issues.apache.org/jira/browse/ZOOKEEPER-3488 and > > > https://issues.apache.org/jira/browse/ZOOKEEPER-3489 > > > I am very sorry that I cannot reply > > > > > > > > > https://lists.apache.org/thread.html/ea0f515bcb994bae54e62584c2c659b83efa4d22e741d4b688c4479b > > > < > > > > > > https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.apache.org_thread.html_ea0f515bcb994bae54e62584c2c659b83efa4d22e741d4b688c4479b-40-253Cuser.zookeeper.apache.org-253E&d=DwMFaQ&c=C3yme8gMkxg_ihJNXS06ZyWk4EJm8LdrrvxQb-Je7sw&r=I7kF79XmGGZ9RLTiorMoewvoyduMXtLcRb1B2epe9cI&m=C_FrvfYh4GtqQesDyKDl4kau6xsDwvLGHAA0IZB9etE&s=mZcIhnGSnLca5JxJbOoKzoTiSuQQZmtyDo_eadoJHcw&e= > > > > > > > Please confirm them and give them CVE IDs. > > > > > > > Honestly they don't sound to me as "Security issues", > > we are talking about not using LOG.isXXXEnabled that is only an > > optimization > > > > if (Log.isWarnEnabled()) { > > Log.warn(xxxxxx) > > } > > > > this construct is used only to save resources and do no evaluate "xxx", > it > > is expected > > that the log subsystem won't output the result of "xxx" if configured > > propertly. > > > > Maybe I am missing something > > > > > That was essentially my read as well - see my comments on the two jira > earlier today. > > Regards, > > Patrick > > > > Regards > > Enrico > > > > > > > > > > > > > > > Thank you very much! > > > Yours sincerely > > > Xiaoqin Fu > > > > > > On Mon, Aug 5, 2019 at 6:23 PM Xiaoqin Fu <[email protected]> > wrote: > > > > > > > Dear Patrick Hunt: > > > > I opened two issues in the JIRA: > > > > https://issues.apache.org/jira/browse/ZOOKEEPER-3488 and > > > > https://issues.apache.org/jira/browse/ZOOKEEPER-3489 > > > > I am very sorry that I cannot reply > > > > > > > > > > https://lists.apache.org/thread.html/ea0f515bcb994bae54e62584c2c659b83efa4d22e741d4b688c4479b > > > > < > > > > > > https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.apache.org_thread.html_ea0f515bcb994bae54e62584c2c659b83efa4d22e741d4b688c4479b-40-253Cuser.zookeeper.apache.org-253E&d=DwMFaQ&c=C3yme8gMkxg_ihJNXS06ZyWk4EJm8LdrrvxQb-Je7sw&r=I7kF79XmGGZ9RLTiorMoewvoyduMXtLcRb1B2epe9cI&m=C_FrvfYh4GtqQesDyKDl4kau6xsDwvLGHAA0IZB9etE&s=mZcIhnGSnLca5JxJbOoKzoTiSuQQZmtyDo_eadoJHcw&e= > > > > > > > > Please confirm them and give them CVE IDs. > > > > > > > > > > > > Thank you very much! > > > > Yours sincerely > > > > Xiaoqin Fu > > > > > > > > > > > > On Fri, Aug 2, 2019 at 11:20 AM Patrick Hunt <[email protected]> > wrote: > > > > > > > >> I know. :-) Once a vulnerability has been publicly disclosed there > is > > no > > > >> value in handling it on the security@ list - that's for undisclosed > > > >> vulnerabilities to be handled in a responsible (private) manner > until > > a > > > fix > > > >> can be released to end users. You can see the process here: > > > >> https://www.apache.org/security/committers.html > > > >> > > > >> Regards, > > > >> > > > >> Patrick > > > >> > > > >> On Fri, Aug 2, 2019 at 10:24 AM Xiaoqin Fu <[email protected]> > > > wrote: > > > >> > > > >>> Dear Patrick Hunt: > > > >>> > > > >>> > > > > > > https://lists.apache.org/thread.html/ea0f515bcb994bae54e62584c2c659b83efa4d22e741d4b688c4479b@ > > > <user.zookeeper.apache.org> > > > >>> < > > > > > > https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.apache.org_thread.html_ea0f515bcb994bae54e62584c2c659b83efa4d22e741d4b688c4479b-40-253Cuser.zookeeper.apache.org-253E&d=DwMFaQ&c=C3yme8gMkxg_ihJNXS06ZyWk4EJm8LdrrvxQb-Je7sw&r=I7kF79XmGGZ9RLTiorMoewvoyduMXtLcRb1B2epe9cI&m=C_FrvfYh4GtqQesDyKDl4kau6xsDwvLGHAA0IZB9etE&s=mZcIhnGSnLca5JxJbOoKzoTiSuQQZmtyDo_eadoJHcw&e= > > > > > > was > > > >>> disclosed by me. > > > >>> > > > >>> Thank you very much! > > > >>> Yours sincerely > > > >>> Xiaoqin Fu > > > >>> > > > >>> > > > >>> On Fri, Aug 2, 2019 at 11:04 AM Patrick Hunt <[email protected]> > > wrote: > > > >>> > > > >>>> Hi Xiaoqin Fu. Thank you for the report. The Apache Software > > > Foundation > > > >>>> and the Apache ZooKeeper project takes security issues very > > seriously. > > > >>>> > > > >>>> Unfortunately this issue has already been publicly disclosed on > both > > > >>>> the dev and user lists: > > > >>>> > > > >>>> > > > > > > https://lists.apache.org/thread.html/ea0f515bcb994bae54e62584c2c659b83efa4d22e741d4b688c4479b@ > > > <user.zookeeper.apache.org> > > > >>>> < > > > > > > https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.apache.org_thread.html_ea0f515bcb994bae54e62584c2c659b83efa4d22e741d4b688c4479b-40-253Cuser.zookeeper.apache.org-253E&d=DwMFaQ&c=C3yme8gMkxg_ihJNXS06ZyWk4EJm8LdrrvxQb-Je7sw&r=I7kF79XmGGZ9RLTiorMoewvoyduMXtLcRb1B2epe9cI&m=C_FrvfYh4GtqQesDyKDl4kau6xsDwvLGHAA0IZB9etE&s=mZcIhnGSnLca5JxJbOoKzoTiSuQQZmtyDo_eadoJHcw&e= > > > > > > > >>>> as such please follow our standard processes documented here: > > > >>>> > > https://cwiki.apache.org/confluence/display/ZOOKEEPER/HowToContribute > > > >>>> < > > > > > > https://urldefense.proofpoint.com/v2/url?u=https-3A__cwiki.apache.org_confluence_display_ZOOKEEPER_HowToContribute&d=DwMFaQ&c=C3yme8gMkxg_ihJNXS06ZyWk4EJm8LdrrvxQb-Je7sw&r=I7kF79XmGGZ9RLTiorMoewvoyduMXtLcRb1B2epe9cI&m=C_FrvfYh4GtqQesDyKDl4kau6xsDwvLGHAA0IZB9etE&s=vhQg3XrZS-o0UME9EYvtoCerG-ZNtJhTtbvo1M_MasM&e= > > > > > > - > > > >>>> the community will work with you to get the issue resolved. > > > >>>> > > > >>>> Regards, > > > >>>> > > > >>>> Patrick > > > >>>> > > > >>>> > > > >>>> On Fri, Aug 2, 2019 at 3:35 AM Fu, Xiaoqin <[email protected]> > > > wrote: > > > >>>> > > > >>>>> Dear developers: > > > >>>>> I am a Ph.D. student at Washington State University. I applied > > > >>>>> dynamic taint analyzer (distTaint) to Apache Zookeeper (version > > > 3.4.11). > > > >>>>> And then I find several bugs, that exist from 3.4.11-3.4.14 and > > > 3.5.5, from > > > >>>>> tainted paths: > > > >>>>> > > > >>>>> 1. In org.apache.zookeeper.server.ZooKeeperServer: > > > >>>>> public ZooKeeperServer(FileTxnSnapLog txnLogFactory, int > > > tickTime, > > > >>>>> int minSessionTimeout, int maxSessionTimeout, > > ZKDatabase > > > >>>>> zkDb) { > > > >>>>> ...... > > > >>>>> LOG.info("Created server with tickTime " + tickTime > > > >>>>> + " minSessionTimeout " + getMinSessionTimeout() > > > >>>>> + " maxSessionTimeout " + getMaxSessionTimeout() > > > >>>>> + " datadir " + txnLogFactory.getDataDir() > > > >>>>> + " snapdir " + txnLogFactory.getSnapDir()); > > > >>>>> ...... > > > >>>>> } > > > >>>>> public void finishSessionInit(ServerCnxn cnxn, boolean valid) > > > >>>>> ...... > > > >>>>> if (!valid) { > > > >>>>> LOG.info("Invalid session 0x" > > > >>>>> + Long.toHexString(cnxn.getSessionId()) > > > >>>>> + " for client " > > > >>>>> + cnxn.getRemoteSocketAddress() > > > >>>>> + ", probably expired"); > > > >>>>> cnxn.sendBuffer(ServerCnxnFactory.closeConn); > > > >>>>> } else { > > > >>>>> LOG.info("Established session 0x" > > > >>>>> + Long.toHexString(cnxn.getSessionId()) > > > >>>>> + " with negotiated timeout " + > > > >>>>> cnxn.getSessionTimeout() > > > >>>>> + " for client " > > > >>>>> + cnxn.getRemoteSocketAddress()); > > > >>>>> cnxn.enableRecv(); > > > >>>>> } > > > >>>>> ...... > > > >>>>> } > > > >>>>> Sensitive information about DataDir, SnapDir, SessionId and > > > >>>>> RemoteSocketAddress may be leaked. I think that it is better to > add > > > >>>>> LOG.isInfoEnabled() conditional statements: > > > >>>>> public ZooKeeperServer(FileTxnSnapLog txnLogFactory, int > > > tickTime, > > > >>>>> int minSessionTimeout, int maxSessionTimeout, > > ZKDatabase > > > >>>>> zkDb) { > > > >>>>> ...... > > > >>>>> if (LOG.isInfoEnabled()) > > > >>>>> LOG.info("Created server with tickTime " + tickTime > > > >>>>> + " minSessionTimeout " + getMinSessionTimeout() > > > >>>>> + " maxSessionTimeout " + getMaxSessionTimeout() > > > >>>>> + " datadir " + txnLogFactory.getDataDir() > > > >>>>> + " snapdir " + txnLogFactory.getSnapDir()); > > > >>>>> ...... > > > >>>>> } > > > >>>>> public void finishSessionInit(ServerCnxn cnxn, boolean valid) { > > > >>>>> ...... > > > >>>>> if (!valid) { > > > >>>>> if (LOG.isInfoEnabled()) > > > >>>>> LOG.info("Invalid session 0x" > > > >>>>> + Long.toHexString(cnxn.getSessionId()) > > > >>>>> + " for client " > > > >>>>> + cnxn.getRemoteSocketAddress() > > > >>>>> + ", probably expired"); > > > >>>>> cnxn.sendBuffer(ServerCnxnFactory.closeConn); > > > >>>>> } else { > > > >>>>> if (LOG.isInfoEnabled()) > > > >>>>> LOG.info("Established session 0x" > > > >>>>> + Long.toHexString(cnxn.getSessionId()) > > > >>>>> + " with negotiated timeout " + > > > >>>>> cnxn.getSessionTimeout() > > > >>>>> + " for client " > > > >>>>> + cnxn.getRemoteSocketAddress()); > > > >>>>> cnxn.enableRecv(); > > > >>>>> } > > > >>>>> ...... > > > >>>>> } > > > >>>>> The LOG.isInfoEnabled() conditional statement already exists in > > > >>>>> org.apache.zookeeper.server.persistence.FileTxnLog: > > > >>>>> public synchronized boolean append(TxnHeader hdr, Record txn) > > throws > > > >>>>> IOException { > > > >>>>> { ...... > > > >>>>> if(LOG.isInfoEnabled()){ > > > >>>>> LOG.info("Creating new log file: " + > > > Util.makeLogName(hdr.getZxid())); > > > >>>>> } > > > >>>>> ...... > > > >>>>> } > > > >>>>> 2. In org.apache.zookeeper.ClientCnxn$SendThread, > > > >>>>> void readResponse(ByteBuffer incomingBuffer) throws > > > >>>>> IOException { > > > >>>>> ...... > > > >>>>> LOG.warn("Got server path " + event.getPath() > > > >>>>> + " which is too short for chroot path " > > > >>>>> + chrootPath); > > > >>>>> ...... > > > >>>>> } > > > >>>>> Sensitive information about event path and chroot path may be > > leaked. > > > >>>>> The LOG.isWarnEnabled() conditional statement should be added: > > > >>>>> void readResponse(ByteBuffer incomingBuffer) throws > > IOException { > > > >>>>> ...... > > > >>>>> if (LOG.isWarnEnabled()) > > > >>>>> LOG.warn("Got server path " + event.getPath() > > > >>>>> + " which is too short for chroot path " > > > >>>>> + chrootPath); > > > >>>>> ...... > > > >>>>> } > > > >>>>> > > > >>>>> Please help me confirm them and give them CVE IDs. > > > >>>>> > > > >>>>> Thank you very much! > > > >>>>> Yours sincerely > > > >>>>> Xiaoqin Fu > > > >>>>> > > > >>>>> > > > > > >
