Uploaded image for project: 'Thrift'
  1. Thrift
  2. THRIFT-3978

Thrift C++ runtime uses assert to prevent overflows, checks sanity only in debug builds

    XMLWordPrintableJSON

    Details

    • Type: Bug
    • Status: Open
    • Priority: Major
    • Resolution: Unresolved
    • Affects Version/s: 0.10.0
    • Fix Version/s: None
    • Component/s: C++ - Library
    • Labels:
    • Environment:

      All

      Description

      Currently there is widespread use of assert in the thrift C++ runtime library. Some of the more disturbing cases are security related, for example checking header sizes. I recommend we eliminate assertions that are only checked in debug mode, and instead throw the appropriate exception, usually a TTransportException with CORRUPTED_DATA as the reason. If we're going to check for an overflow or a buffer overrun, we should do so in debug and release modes. Further, assertions are not easily tested whereas exceptions are.

      In THRIFT-3873 apache::thrift::transport::safe_numeric_cast was added, so I also suggest changing static_cast to safe_numeric_cast where appropriate throughout the transport code to catch any overflow errors.

      Another location where assert is used liberally is inside the posix Mutex implementation.

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                jking3 James E. King III
                Reporter:
                jking3 James E. King III
              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated: