You are viewing a plain text version of this content. The canonical link for it is here.
Posted to server-dev@james.apache.org by rc...@apache.org on 2020/02/12 04:14:35 UTC

[james-project] 08/09: JAMES-3042 ImapRequestFrameDecoder::newCumulationBuffer should sanitize negative size

This is an automated email from the ASF dual-hosted git repository.

rcordier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/james-project.git

commit 365dd4a9dc7c6e0034438d64ef7f81a54575760b
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Mon Feb 10 10:44:40 2020 +0700

    JAMES-3042 ImapRequestFrameDecoder::newCumulationBuffer should sanitize negative size
    
    NotEnoughDataException is using -1 as a "unknown" marker, making this code path
    potentially buggy.
---
 .../james/imapserver/netty/ImapRequestFrameDecoder.java | 15 ++++++++-------
 .../imapserver/netty/ImapRequestFrameDecoderTest.java   | 17 +++++++++++++++++
 2 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/ImapRequestFrameDecoder.java b/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/ImapRequestFrameDecoder.java
index 801f6d7..c8e35de 100644
--- a/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/ImapRequestFrameDecoder.java
+++ b/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/ImapRequestFrameDecoder.java
@@ -42,6 +42,8 @@ import org.jboss.netty.channel.ChannelPipeline;
 import org.jboss.netty.channel.ChannelStateEvent;
 import org.jboss.netty.handler.codec.frame.FrameDecoder;
 
+import com.google.common.annotations.VisibleForTesting;
+
 /**
  * {@link FrameDecoder} which will decode via and {@link ImapDecoder} instance
  */
@@ -50,7 +52,8 @@ public class ImapRequestFrameDecoder extends FrameDecoder implements NettyConsta
     private final ImapDecoder decoder;
     private final int inMemorySizeLimit;
     private final int literalSizeLimit;
-    private static final String NEEDED_DATA = "NEEDED_DATA";
+    @VisibleForTesting
+    static final String NEEDED_DATA = "NEEDED_DATA";
     private static final String STORED_DATA = "STORED_DATA";
     private static final String WRITTEN_DATA = "WRITTEN_DATA";
     private static final String OUTPUT_STREAM = "OUTPUT_STREAM";
@@ -227,13 +230,11 @@ public class ImapRequestFrameDecoder extends FrameDecoder implements NettyConsta
             @SuppressWarnings("unchecked")
             int size = (Integer) sizeAsObject;
 
-            if (inMemorySizeLimit > 0) {
-                return ChannelBuffers.dynamicBuffer(Math.min(size, inMemorySizeLimit), ctx.getChannel().getConfig().getBufferFactory());
-            } else {
+            if (size > 0) {
+                int sanitizedInMemorySizeLimit = Math.max(0, inMemorySizeLimit);
+                int sanitizedSize = Math.min(sanitizedInMemorySizeLimit, size);
 
-                if (size > 0) {
-                    return ChannelBuffers.dynamicBuffer(size, ctx.getChannel().getConfig().getBufferFactory());
-                }
+                return ChannelBuffers.dynamicBuffer(sanitizedSize, ctx.getChannel().getConfig().getBufferFactory());
             }
         }
         return super.newCumulationBuffer(ctx, minimumCapacity);
diff --git a/server/protocols/protocols-imap4/src/test/java/org/apache/james/imapserver/netty/ImapRequestFrameDecoderTest.java b/server/protocols/protocols-imap4/src/test/java/org/apache/james/imapserver/netty/ImapRequestFrameDecoderTest.java
index 1c66c83..8dffa0b 100644
--- a/server/protocols/protocols-imap4/src/test/java/org/apache/james/imapserver/netty/ImapRequestFrameDecoderTest.java
+++ b/server/protocols/protocols-imap4/src/test/java/org/apache/james/imapserver/netty/ImapRequestFrameDecoderTest.java
@@ -20,6 +20,7 @@
 package org.apache.james.imapserver.netty;
 
 
+import static org.apache.james.imapserver.netty.ImapRequestFrameDecoder.NEEDED_DATA;
 import static org.assertj.core.api.Assertions.assertThatCode;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
@@ -60,4 +61,20 @@ class ImapRequestFrameDecoderTest {
         assertThatCode(() -> testee.newCumulationBuffer(channelHandler, 36))
             .doesNotThrowAnyException();
     }
+
+    @Test
+    void newCumulationBufferShouldNotThrowOnNegativeSize() {
+        ChannelHandlerContext channelHandler = mock(ChannelHandlerContext.class);
+        Channel channel = mock(Channel.class);
+        ChannelConfig channelConfig = mock(ChannelConfig.class);
+
+        when(channelConfig.getBufferFactory()).thenReturn(mock(ChannelBufferFactory.class));
+        when(channelHandler.getChannel()).thenReturn(channel);
+        when(channel.getConfig()).thenReturn(channelConfig);
+
+        when(channelHandler.getAttachment()).thenReturn(ImmutableMap.<String, Object>of(NEEDED_DATA, -1));
+
+        assertThatCode(() -> testee.newCumulationBuffer(channelHandler, 36))
+            .doesNotThrowAnyException();
+    }
 }
\ No newline at end of file


---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org