zookeeper-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jürgen Wagner (JIRA) <j...@apache.org>
Subject [jira] [Commented] (ZOOKEEPER-3504) An information leakage from FileTxnSnapLog to log:
Date Tue, 13 Aug 2019 08:51:00 GMT

    [ https://issues.apache.org/jira/browse/ZOOKEEPER-3504?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16905961#comment-16905961
] 

Jürgen Wagner commented on ZOOKEEPER-3504:
------------------------------------------

Generally, either way works. The fact that one part of the code has the LOG.isDebugEnabled()
check, while another doesn't, is something I would place in the domain of personal styles.
The issue you raised does not sound like a security problem in any case.

It is beneficial to use the check if the enclosed statement would perform some computation
which may not be necessary in case of the debug mode being disabled. In the easiest case,
that's something like the string concatenation you quoted above. You still see this in the
code in some places

[https://github.com/apache/zookeeper/blob/master/zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxn.java]

However, the preferred method of avoiding the string concatenation is the use of "{}" parametrized
log strings.

PS: The fact that Zookeeper is used in productive systems does not mean you want "debug" mode
switched on there. It also does not tell you how operators of such instances actually secure
their instances and log services. That's an operational topic.

> An information leakage from FileTxnSnapLog to log:
> --------------------------------------------------
>
>                 Key: ZOOKEEPER-3504
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-3504
>             Project: ZooKeeper
>          Issue Type: Bug
>          Components: security, server
>    Affects Versions: 3.4.11, 3.4.12, 3.4.13, 3.5.5, 3.4.14
>            Reporter: xiaoqin.fu
>            Priority: Major
>
> In org.apache.zookeeper.server.persistence.FileTxnSnapLog, the statement LOG.debug don't
have LOG controls:
>     public void processTransaction(TxnHeader hdr,DataTree dt,
>             Map<Long, Integer> 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<Long, Integer> 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);
>         }  
> 		......
>     } 



--
This message was sent by Atlassian JIRA
(v7.6.14#76016)

Mime
View raw message