zookeeper-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From hanm <...@git.apache.org>
Subject [GitHub] zookeeper pull request #179: ZOOKEEPER-2693: DOS attack on wchp/wchc four le...
Date Tue, 21 Feb 2017 18:05:41 GMT
Github user hanm commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/179#discussion_r102275734
  
    --- Diff: src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java ---
    @@ -267,10 +267,17 @@ private boolean checkFourLetterWord(final Channel channel,
         {
             // We take advantage of the limited size of the length to look
             // for cmds. They are all 4-bytes which fits inside of an int
    -        String cmd = FourLetterCommands.getCmdMapView().get(len);
    -        if (cmd == null) {
    +        if (!FourLetterCommands.isKnown(len)) {
                 return false;
             }
    +
    +        // ZOOKEEPER-2693: don't execute 4lw if it's not enabled.
    +        String cmd = FourLetterCommands.getCommandString(len);
    +        if (!FourLetterCommands.isEnabled(cmd)) {
    +            LOG.debug("Command {} is not executed because it is not white listed.", cmd);
    +            return true;
    --- End diff --
    
    @rakeshadr hmmmm I think we did not even close server sockets today for four letter words
in general, see 
    https://github.com/apache/zookeeper/blob/master/src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java#L343
    and
    https://github.com/apache/zookeeper/blob/master/src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java#L388
    
    This is not a problem in general for the command line interface because in that case client
socket will close first and then server socket will close as a result of client socket close...
however if someone writes a client that opens and holds a socket then server will not close
the socket even after 4lw finish execution. This probably is a by design as it allows clients
to pipe 4lw commands w/o re-opening sockets but I see this is another potential point of vulnerability
where a server could run out of sockets..
    
    I think we should probably close the sockets in the two links I posted in the beginning.
Let me know what you think @rakeshadr.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message