You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by "Roger Meier (JIRA)" <ji...@apache.org> on 2013/05/05 23:40:15 UTC

[jira] [Resolved] (THRIFT-1932) TFileTransport::readEvent() casts values read from input stream into a pointer and then dereferences it.

     [ https://issues.apache.org/jira/browse/THRIFT-1932?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Roger Meier resolved THRIFT-1932.
---------------------------------

    Resolution: Fixed
      Assignee: Roger Meier

Thanks Hugo!
committed
                
> TFileTransport::readEvent() casts values read from input stream into a pointer and then dereferences it.
> --------------------------------------------------------------------------------------------------------
>
>                 Key: THRIFT-1932
>                 URL: https://issues.apache.org/jira/browse/THRIFT-1932
>             Project: Thrift
>          Issue Type: Bug
>          Components: C++ - Library
>    Affects Versions: 0.9
>         Environment: Hardened Gentoo amd64 Linux 
>            Reporter: Hugo Mildenberger
>            Assignee: Roger Meier
>         Attachments: thrift-0.9.0-fix-type-punned-pointer-warning.patch
>
>
> The Compilation of thrift-0.9.0 ended with the following warnings:
>  * QA Notice: Package triggers severe warnings which indicate that it
>  *            may exhibit random runtime failures.
>  * src/thrift/transport/TFileTransport.cpp:715:56: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
>  * src/thrift/transport/TFileTransport.cpp:726:84: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
> When looking into src/thrift/transport/TFileTransport.cpp:715 ...
> 711         readState_.eventSizeBuff_[readState_.eventSizeBuffPos_++] =
> 712           readBuff_[readState_.bufferPtr_++];
> 713         if (readState_.eventSizeBuffPos_ == 4) {
> 714           // 0 length event indicates padding
> 715           if (*((uint32_t *)(readState_.eventSizeBuff_)) == 0) {
> 716             //            T_DEBUG_L(1, "Got padding");
> 717             readState_.resetState(readState_.lastDispatchPtr_);
> 718             continue;
> 719           }
> ... it becomes obvious that the four values casted into a  pointer were previously read from the input stream by ::read() in line 661: 
> 661       readState_.bufferLen_ = ::read(fd_, readBuff_, readBuffSize_);
> Same issue here:
> 725           readState_.event_ = new eventInfo();
> 726           readState_.event_->eventSize_ = \*((uint32_t*)(readState_.eventSizeBuff_));
> 727 
> While it is a two minute job fix this problem, the method TFileTransport::readEvent() looks so fragile that a clean rewrite appears to be more appropriate. 

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira