[jira] [Commented] (THRIFT-4362) Missing size-check can lead to huge memory allocation

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/THRIFT-4362?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16218845#comment-16218845
 ] 

ASF GitHub Bot commented on THRIFT-4362:


Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/1398
  
We only have resources on this project to handle one release stream (and 
even then it's a challenge), so we're starting the process of making 0.11.0 
with all the changes since 0.10.0 right now.


> Missing size-check can lead to huge memory allocation
> -
>
> Key: THRIFT-4362
> URL: https://issues.apache.org/jira/browse/THRIFT-4362
> Project: Thrift
>  Issue Type: Bug
>  Components: Java - Library
>Affects Versions: 0.9.3, 0.10.0
>Reporter: Christian Ciach
>Assignee: James E. King, III
> Fix For: 0.11.0
>
> Attachments: check-size.patch
>
>
> In some cases the method 
> {{org.apache.thrift.protocol.TBinaryProtocol.readStringBody(int size)}} gets 
> called with a "size" parameter that has not been validated by the existing 
> method {{checkStringReadLength(int size)}}.
> This is true if the method is called by {{readMessageBegin()}} of the same 
> class. The method {{readString()}} checks the size correctly before calling 
> {{readStringBody(int size)}}.
> Since the methods {{readStringBody(int size)}} and {{readMessageBegin()}} are 
> public, there may be other callers who don't check the size correctly.
> We encountered this issue in production several times. Because of this we are 
> currently using our own patched version of libthrift-0.9.3. The patch is 
> attached, but it is surely not the best solution, because with this patch the 
> size may be checked twice, depending on the caller.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (THRIFT-4362) Missing size-check can lead to huge memory allocation

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/THRIFT-4362?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16218838#comment-16218838
 ] 

ASF GitHub Bot commented on THRIFT-4362:


Github user ChristianCiach commented on the issue:

https://github.com/apache/thrift/pull/1398
  
So there won't be a version 0.9.4 or 0.10.1?


> Missing size-check can lead to huge memory allocation
> -
>
> Key: THRIFT-4362
> URL: https://issues.apache.org/jira/browse/THRIFT-4362
> Project: Thrift
>  Issue Type: Bug
>  Components: Java - Library
>Affects Versions: 0.9.3, 0.10.0
>Reporter: Christian Ciach
>Assignee: James E. King, III
> Fix For: 0.11.0
>
> Attachments: check-size.patch
>
>
> In some cases the method 
> {{org.apache.thrift.protocol.TBinaryProtocol.readStringBody(int size)}} gets 
> called with a "size" parameter that has not been validated by the existing 
> method {{checkStringReadLength(int size)}}.
> This is true if the method is called by {{readMessageBegin()}} of the same 
> class. The method {{readString()}} checks the size correctly before calling 
> {{readStringBody(int size)}}.
> Since the methods {{readStringBody(int size)}} and {{readMessageBegin()}} are 
> public, there may be other callers who don't check the size correctly.
> We encountered this issue in production several times. Because of this we are 
> currently using our own patched version of libthrift-0.9.3. The patch is 
> attached, but it is surely not the best solution, because with this patch the 
> size may be checked twice, depending on the caller.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (THRIFT-4362) Missing size-check can lead to huge memory allocation

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/THRIFT-4362?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16218532#comment-16218532
 ] 

ASF GitHub Bot commented on THRIFT-4362:


Github user asfgit closed the pull request at:

https://github.com/apache/thrift/pull/1398


> Missing size-check can lead to huge memory allocation
> -
>
> Key: THRIFT-4362
> URL: https://issues.apache.org/jira/browse/THRIFT-4362
> Project: Thrift
>  Issue Type: Bug
>  Components: Java - Library
>Affects Versions: 0.9.3, 0.10.0
>Reporter: Christian Ciach
>Assignee: James E. King, III
> Fix For: 0.11.0
>
> Attachments: check-size.patch
>
>
> In some cases the method 
> {{org.apache.thrift.protocol.TBinaryProtocol.readStringBody(int size)}} gets 
> called with a "size" parameter that has not been validated by the existing 
> method {{checkStringReadLength(int size)}}.
> This is true if the method is called by {{readMessageBegin()}} of the same 
> class. The method {{readString()}} checks the size correctly before calling 
> {{readStringBody(int size)}}.
> Since the methods {{readStringBody(int size)}} and {{readMessageBegin()}} are 
> public, there may be other callers who don't check the size correctly.
> We encountered this issue in production several times. Because of this we are 
> currently using our own patched version of libthrift-0.9.3. The patch is 
> attached, but it is surely not the best solution, because with this patch the 
> size may be checked twice, depending on the caller.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (THRIFT-4362) Missing size-check can lead to huge memory allocation

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/THRIFT-4362?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16218514#comment-16218514
 ] 

ASF GitHub Bot commented on THRIFT-4362:


Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/1398
  
We don't re-release previous versions.  You will need to apply the patch to 
your own copy of a previous version.


> Missing size-check can lead to huge memory allocation
> -
>
> Key: THRIFT-4362
> URL: https://issues.apache.org/jira/browse/THRIFT-4362
> Project: Thrift
>  Issue Type: Bug
>  Components: Java - Library
>Affects Versions: 0.9.3, 0.10.0
>Reporter: Christian Ciach
>Assignee: James E. King, III
> Attachments: check-size.patch
>
>
> In some cases the method 
> {{org.apache.thrift.protocol.TBinaryProtocol.readStringBody(int size)}} gets 
> called with a "size" parameter that has not been validated by the existing 
> method {{checkStringReadLength(int size)}}.
> This is true if the method is called by {{readMessageBegin()}} of the same 
> class. The method {{readString()}} checks the size correctly before calling 
> {{readStringBody(int size)}}.
> Since the methods {{readStringBody(int size)}} and {{readMessageBegin()}} are 
> public, there may be other callers who don't check the size correctly.
> We encountered this issue in production several times. Because of this we are 
> currently using our own patched version of libthrift-0.9.3. The patch is 
> attached, but it is surely not the best solution, because with this patch the 
> size may be checked twice, depending on the caller.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (THRIFT-4362) Missing size-check can lead to huge memory allocation

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/THRIFT-4362?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16216453#comment-16216453
 ] 

ASF GitHub Bot commented on THRIFT-4362:


GitHub user ChristianCiach opened a pull request:

https://github.com/apache/thrift/pull/1398

THRIFT-4362 check "read length" in readStringBody(int)

This fixes THRIFT-4362.

If possible, please port this fix to previous versions. 

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/ChristianCiach/thrift THRIFT-4362

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/thrift/pull/1398.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1398


commit 37dfb6cfda585aec82226f62814eb00ed609c3fe
Author: christianc 
Date:   2017-10-24T07:25:20Z

THRIFT-4362 check "read length" in readStringBody(int)




> Missing size-check can lead to huge memory allocation
> -
>
> Key: THRIFT-4362
> URL: https://issues.apache.org/jira/browse/THRIFT-4362
> Project: Thrift
>  Issue Type: Bug
>  Components: Java - Library
>Affects Versions: 0.9.3, 0.10.0
>Reporter: Christian Ciach
>Assignee: James E. King, III
> Attachments: check-size.patch
>
>
> In some cases the method 
> {{org.apache.thrift.protocol.TBinaryProtocol.readStringBody(int size)}} gets 
> called with a "size" parameter that has not been validated by the existing 
> method {{checkStringReadLength(int size)}}.
> This is true if the method is called by {{readMessageBegin()}} of the same 
> class. The method {{readString()}} checks the size correctly before calling 
> {{readStringBody(int size)}}.
> Since the methods {{readStringBody(int size)}} and {{readMessageBegin()}} are 
> public, there may be other callers who don't check the size correctly.
> We encountered this issue in production several times. Because of this we are 
> currently using our own patched version of libthrift-0.9.3. The patch is 
> attached, but it is surely not the best solution, because with this patch the 
> size may be checked twice, depending on the caller.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (THRIFT-4362) Missing size-check can lead to huge memory allocation

[Christian Ciach (JIRA)]

[ 
https://issues.apache.org/jira/browse/THRIFT-4362?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16212477#comment-16212477
 ] 

Christian Ciach commented on THRIFT-4362:
-

Do you think I can submit this patch as-is or do you have any suggestions for 
improvement?

> Missing size-check can lead to huge memory allocation
> -
>
> Key: THRIFT-4362
> URL: https://issues.apache.org/jira/browse/THRIFT-4362
> Project: Thrift
>  Issue Type: Bug
>  Components: Java - Library
>Affects Versions: 0.9.3, 0.10.0
>Reporter: Christian Ciach
>Assignee: James E. King, III
> Attachments: check-size.patch
>
>
> In some cases the method 
> {{org.apache.thrift.protocol.TBinaryProtocol.readStringBody(int size)}} gets 
> called with a "size" parameter that has not been validated by the existing 
> method {{checkStringReadLength(int size)}}.
> This is true if the method is called by {{readMessageBegin()}} of the same 
> class. The method {{readString()}} checks the size correctly before calling 
> {{readStringBody(int size)}}.
> Since the methods {{readStringBody(int size)}} and {{readMessageBegin()}} are 
> public, there may be other callers who don't check the size correctly.
> We encountered this issue in production several times. Because of this we are 
> currently using our own patched version of libthrift-0.9.3. The patch is 
> attached, but it is surely not the best solution, because with this patch the 
> size may be checked twice, depending on the caller.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (THRIFT-4362) Missing size-check can lead to huge memory allocation

[James E. King, III (JIRA)]

[ 
https://issues.apache.org/jira/browse/THRIFT-4362?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16210988#comment-16210988
 ] 

James E. King, III commented on THRIFT-4362:


Thanks for the patch, once the CI build is stable again (hopefully today) I 
will submit a pull request on your behalf unless you want to do it yourself.

> Missing size-check can lead to huge memory allocation
> -
>
> Key: THRIFT-4362
> URL: https://issues.apache.org/jira/browse/THRIFT-4362
> Project: Thrift
>  Issue Type: Bug
>  Components: Java - Library
>Affects Versions: 0.9.3, 0.10.0
>Reporter: Christian Ciach
> Attachments: check-size.patch
>
>
> In some cases the method 
> {{org.apache.thrift.protocol.TBinaryProtocol.readStringBody(int size)}} gets 
> called with a "size" parameter that has not been validated by the existing 
> method {{checkStringReadLength(int size)}}.
> This is true if the method is called by {{readMessageBegin()}} of the same 
> class. The method {{readString()}} checks the size correctly before calling 
> {{readStringBody(int size)}}.
> Since the methods {{readStringBody(int size)}} and {{readMessageBegin()}} are 
> public, there may be other callers who don't check the size correctly.
> We encountered this issue in production several times. Because of this we are 
> currently using our own patched version of libthrift-0.9.3. The patch is 
> attached, but it is surely not the best solution, because with this patch the 
> size may be checked twice, depending on the caller.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)