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 > > >>>>> > > >>>>> > > >
