[
https://issues.apache.org/jira/browse/THRIFT-5854?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17926191#comment-17926191
]
Jens Geyer commented on THRIFT-5854:
------------------------------------
Good catch, thanks!
> TCompactProtocol readString checks maxMessageSize at wrong position and off
> by one
> ----------------------------------------------------------------------------------
>
> Key: THRIFT-5854
> URL: https://issues.apache.org/jira/browse/THRIFT-5854
> Project: Thrift
> Issue Type: Bug
> Components: C++ - Library
> Affects Versions: 0.14.0
> Reporter: Maximilian Bandle
> Assignee: Maximilian Bandle
> Priority: Major
> Fix For: 0.22.0
>
> Time Spent: 20m
> Remaining Estimate: 0h
>
> While changing the casts from old style casts, I noticed that there might be
> a bug in readString.
> The current code flow follows the logic:
> # Read String Length
> # Check if Length is positive and smaller then stringLimit
> # Make sure buffer is large enough and optionally reallocate
> # Read into buffer and assign
> # Check maxMessageSize and throw if exhausted
> # Return length
> However, to avoid potentially large allocations and failing in step 4 read,
> we can move the check of maxMessageSize before step 3 to ensure the buffer is
> not enlarged.
>
> Furthermore in the current check, there is a subtle bug, since we check if
> there is still enough space to both read the string and the varint storing
> the length. However, we have read the varint already and are thus consumed
> the bytes already. This could lead to the check wrongly failing.
> A changed testcase covers this behavior.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
