Re: An Apache Zookeeper Security Vulnerability

2019-08-12 Thread David Mollitor
If logging is higher than DEBUG level, the message will not print.  The
Log.debug() method will check the log level internally.  Adding the
external check is simply a potential performance optimization.

Thanks.

On Mon, Aug 12, 2019, 10:41 PM Xiaoqin Fu  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 a security vulnerability, that exists from 3.4.11-3.4.14 and 3.5.5,
> from tainted paths.
>
> An information leakage from FileTxnSnapLog to log:
> In org.apache.zookeeper.server.persistence.FileTxnSnapLog, the statement
> LOG.debug don't have LOG controls:
> public void processTransaction(TxnHeader hdr,DataTree dt,
> Map sessions, Record txn)
> throws KeeperException.NoNodeException {
> ..
> if (rc.err != Code.OK.intValue()) {
> LOG.debug("Ignoring processTxn failure hdr:" + hdr.getType()
> + ", error: " + rc.err + ", path: " + rc.path);
> }
> ..
> }
>
> Sensitive information about hdr type or rc path was leaked. The conditional
> statement LOG.isDebugEnabled() should be added:
> public void processTransaction(TxnHeader hdr,DataTree dt,
> Map sessions, Record txn)
> throws KeeperException.NoNodeException {
> ..
> if (rc.err != Code.OK.intValue()) {
> if (LOG.isDebugEnabled())
> LOG.debug("Ignoring processTxn failure hdr:" + hdr.getType()
> + ", error: " + rc.err + ", path: " + rc.path);
> }
> ..
> }
> In JIRA, it is at https://issues.apache.org/jira/browse/ZOOKEEPER-3504
> Please help me confirm it.
>
> Thank you very much!
> Yours sincerely
> Xiaoqin Fu
>


Re: An Apache Zookeeper Security Vulnerability

2019-08-10 Thread Norbert Kalmar
Hello Xiaoqin,

My understanding is that log guards is used for performance reasons. I
don't see how it can prevent information leakage.

I'd also like to add, that please use the security mailing list first if
you think you found a CVE. - secur...@zookeeper.apache.org
More info here:
https://zookeeper.apache.org/security.html

Thank you!

Regards,
Norbert

On Sat, Aug 10, 2019 at 1:31 AM Patrick Hunt  wrote:

> On Fri, Aug 9, 2019 at 9:34 AM Enrico Olivelli 
> wrote:
>
> > Those points do not seem a security issue
> >
> >
> Agree. First off the data is not sensitive. Also it's debug level and
> logged on the server. See
> https://issues.apache.org/jira/browse/ZOOKEEPER-3488 - similar situation
> although in this case debug is not the default - user would actively have
> to turn this on.
>
> Patrick
>
>
> >
> > Enrico
> >
> >
> > Il ven 9 ago 2019, 17:52 Fu, Xiaoqin  ha scritto:
> >
> > > 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 a security vulnerability, that exists from
> 3.4.11-3.4.14
> > > and 3.5.5, from tainted paths.
> > >
> > > Possible information leakage from FileTxnSnapLog to log without LOG
> > > control LOG.isDebugEnabled():
> > > In org.apache.zookeeper.server.persistence.FileTxnSnapLog, the
> statement
> > > LOG.debug don't have LOG controls:
> > > public void processTransaction(TxnHeader hdr,DataTree dt,
> > > Map sessions, Record txn)
> > > throws KeeperException.NoNodeException {
> > > ..
> > > if (rc.err != Code.OK.intValue()) {
> > > LOG.debug("Ignoring processTxn failure hdr:" +
> hdr.getType()
> > > + ", error: " + rc.err + ", path: " + rc.path);
> > > }
> > > ..
> > > }
> > >
> > > Sensitive information about hdr type or rc path may be leaked. The
> > > conditional statement LOG.isDebugEnabled() should be added:
> > > public void processTransaction(TxnHeader hdr,DataTree dt,
> > > Map sessions, Record txn)
> > > throws KeeperException.NoNodeException {
> > > ..
> > > if (rc.err != Code.OK.intValue()) {
> > > if (LOG.isDebugEnabled())
> > > LOG.debug("Ignoring processTxn failure hdr:" + hdr.getType()
> > > + ", error: " + rc.err + ", path: " + rc.path);
> > > }
> > > ..
> > > }
> > > Please help me confirm it and give it a CVE ID.
> > >
> > > Thank you very much!
> > > Yours sincerely
> > > Xiaoqin Fu
> > >
> > >
> >
>


Re: An Apache Zookeeper Security Vulnerability

2019-08-09 Thread Patrick Hunt
On Fri, Aug 9, 2019 at 9:34 AM Enrico Olivelli  wrote:

> Those points do not seem a security issue
>
>
Agree. First off the data is not sensitive. Also it's debug level and
logged on the server. See
https://issues.apache.org/jira/browse/ZOOKEEPER-3488 - similar situation
although in this case debug is not the default - user would actively have
to turn this on.

Patrick


>
> Enrico
>
>
> Il ven 9 ago 2019, 17:52 Fu, Xiaoqin  ha scritto:
>
> > 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 a security vulnerability, that exists from 3.4.11-3.4.14
> > and 3.5.5, from tainted paths.
> >
> > Possible information leakage from FileTxnSnapLog to log without LOG
> > control LOG.isDebugEnabled():
> > In org.apache.zookeeper.server.persistence.FileTxnSnapLog, the statement
> > LOG.debug don't have LOG controls:
> > public void processTransaction(TxnHeader hdr,DataTree dt,
> > Map sessions, Record txn)
> > throws KeeperException.NoNodeException {
> > ..
> > if (rc.err != Code.OK.intValue()) {
> > LOG.debug("Ignoring processTxn failure hdr:" + hdr.getType()
> > + ", error: " + rc.err + ", path: " + rc.path);
> > }
> > ..
> > }
> >
> > Sensitive information about hdr type or rc path may be leaked. The
> > conditional statement LOG.isDebugEnabled() should be added:
> > public void processTransaction(TxnHeader hdr,DataTree dt,
> > Map sessions, Record txn)
> > throws KeeperException.NoNodeException {
> > ..
> > if (rc.err != Code.OK.intValue()) {
> > if (LOG.isDebugEnabled())
> > LOG.debug("Ignoring processTxn failure hdr:" + hdr.getType()
> > + ", error: " + rc.err + ", path: " + rc.path);
> > }
> > ..
> > }
> > Please help me confirm it and give it a CVE ID.
> >
> > Thank you very much!
> > Yours sincerely
> > Xiaoqin Fu
> >
> >
>


Re: An Apache Zookeeper Security Vulnerability

2019-08-09 Thread Enrico Olivelli
Those points do not seem a security issue


Enrico


Il ven 9 ago 2019, 17:52 Fu, Xiaoqin  ha scritto:

> 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 a security vulnerability, that exists from 3.4.11-3.4.14
> and 3.5.5, from tainted paths.
>
> Possible information leakage from FileTxnSnapLog to log without LOG
> control LOG.isDebugEnabled():
> In org.apache.zookeeper.server.persistence.FileTxnSnapLog, the statement
> LOG.debug don't have LOG controls:
> public void processTransaction(TxnHeader hdr,DataTree dt,
> Map sessions, Record txn)
> throws KeeperException.NoNodeException {
> ..
> if (rc.err != Code.OK.intValue()) {
> LOG.debug("Ignoring processTxn failure hdr:" + hdr.getType()
> + ", error: " + rc.err + ", path: " + rc.path);
> }
> ..
> }
>
> Sensitive information about hdr type or rc path may be leaked. The
> conditional statement LOG.isDebugEnabled() should be added:
> public void processTransaction(TxnHeader hdr,DataTree dt,
> Map sessions, Record txn)
> throws KeeperException.NoNodeException {
> ..
> if (rc.err != Code.OK.intValue()) {
> if (LOG.isDebugEnabled())
> LOG.debug("Ignoring processTxn failure hdr:" + hdr.getType()
> + ", error: " + rc.err + ", path: " + rc.path);
> }
> ..
> }
> Please help me confirm it and give it a CVE ID.
>
> Thank you very much!
> Yours sincerely
> Xiaoqin Fu
>
>