[tao-bugs] TAO_HTTP_Handler, HTTP_Handler.cpp: HTTP_Handler fails to fetch IOR due to bugs

BJovke . bjovan at gmail.com
Wed Apr 20 04:27:24 CDT 2016


:)) Well, I've just noticed that initial "buf" is also not zero terminated.
For strstr this is critical.
So here is the fix with this correction included:

   SAMPLE FIX/WORKAROUND:

int
TAO_HTTP_Reader::receive_reply (void)
{
  size_t num_recvd = 0;
  char buf [MTU+1];
  char *buf_ptr = 0;
  size_t bytes_read = 0;

  // Receive the first MTU bytes and strip the header off.
  // Note that we assume that the header will fit into MTU bytes.
  if (peer ().recv_n (buf, MTU, 0, &num_recvd) >= 0)
    {
      // Zero terminate buf.
  buf[num_recvd]=0;
      //Make sure that response type is 200 OK
  // It could be "IOR:" also!!!
      if (ACE_OS::strstr (buf,"200 OK") == 0)
{
// If response is pure IOR string it should begin with "IOR:" without any
white spaces before it
// This could be modified in future to skip leading white spaces if there
is a possibility that there are white spaces before "IOR:" string
if (*buf == 0 || ACE_OS::strstr (buf,"IOR:") != buf) // *buf==0 is here
because strstr returns buf when buf is empty string
TAOLIB_ERROR_RETURN ((LM_ERROR,
"TAO (%P|%t) - HTTP_Reader::receive_reply, Response is not 200 OK nor
IOR:\n" ), -1);
}

      // Search for the header termination string "\r\n\r\n", or "\n\n". If
      // found, move past it to get to the data portion.
      if ((buf_ptr = ACE_OS::strstr (buf,"\r\n\r\n")) != 0)
        buf_ptr += 4;
      else if ((buf_ptr = ACE_OS::strstr (buf, "\n\n")) != 0)     //for
compatibility with JAWS
        buf_ptr += 2;
      else
        buf_ptr = buf;

      // Determine number of data bytes read. This is equal to the
      // total bytes read minus number of header bytes.
      bytes_read = num_recvd - (buf_ptr - buf);

    }
  else
    {
      TAOLIB_ERROR_RETURN ((LM_ERROR,
                         "TAO (%P|%t) - HTTP_Reader::receive_reply, error
while reading header\n"), -1);
    }

  // ***************************************************************
  // At this point, we have stripped off the header and are ready to
  // process data. buf_ptr points to the data

  ACE_Message_Block* temp = 0;
  ACE_Message_Block* curr = this->mb_;

  ACE_NEW_RETURN (temp,
                  ACE_Message_Block (bytes_read + 1), // Added "+1" to make
room for byte 0 to make 0 terminated string.
                  -1);
  curr->cont (temp);
  curr = curr->cont ();

  // Copy over all the data bytes into our message buffer.
  if (curr->copy (buf_ptr, bytes_read) == -1)
    {
      TAOLIB_ERROR_RETURN ((LM_ERROR, "TAO (%P|%t) -
HTTP_Reader::receive_reply, error copying data into Message_Block\n"), -1);
    }

// 0 - terminate string
*curr->wr_ptr () = 0;
  curr->wr_ptr (1);

  // read the rest of the data into a number of ACE_Message_Blocks and
  // chain them together in a link list fashion
  num_recvd = 0;

  do
  {
    if (curr->space () == 0)
    {
      ACE_NEW_RETURN (temp,
                      ACE_Message_Block (MTU + 1), // Added "+1" to make
room for byte 0 to make 0 terminated string.
                      -1);
      curr->cont (temp);
      curr = curr->cont ();
    }

  if (peer ().recv_n (curr->wr_ptr (), curr->space (), 0, &num_recvd) >= 0)
    {
      // Move the write pointer
      curr->wr_ptr (num_recvd);

// 0 - terminate string
*curr->wr_ptr () = 0;
      curr->wr_ptr (1);

      // Increment bytes_read
      bytes_read += num_recvd;

    }
  else
    {
      TAOLIB_ERROR_RETURN ((LM_ERROR,
                         "TAO (%P|%t) - HTTP_Reader::receive_reply, Error
while reading header\n"), -1);
    }
  } while (num_recvd != 0);

  // Set the byte count to number of bytes received
  this->bytecount_ = bytes_read;

  return 0;
}


2016-04-20 11:08 GMT+02:00 BJovke . <bjovan at gmail.com>:

>     TAO VERSION: 2.3.0
>     ACE VERSION: 6.3.0
>
>     HOST MACHINE and OPERATING SYSTEM:
>         Windows 7, Winsock2
>
>     COMPILER NAME AND VERSION (AND PATCHLEVEL):
>         Visual studio 2012 express VC11 version 17.00.61030 for x86
>
>     THE $ACE_ROOT/ace/config.h FILE [if you use a link to a platform-
>     specific file, simply state which one]:
>         config-win32.h
>
>     THE $ACE_ROOT/include/makeinclude/platform_macros.GNU FILE [if you
>     use a link to a platform-specific file, simply state which one
>     (unless this isn't used in this case, e.g., with Microsoft Visual
>     C++)]:
>         MSVC used
>
>     CONTENTS OF $ACE_ROOT/bin/MakeProjectCreator/config/default.features
>     (used by MPC when you generate your own makefiles):
>         -
>
>     AREA/CLASS/EXAMPLE AFFECTED:
>         Failed fetching of IOR via HTTP.
>
>     DOES THE PROBLEM AFFECT:
>         COMPILATION?
>             No.
>         LINKING?
>             No.
>         EXECUTION?
>             Yes, behavior is wrong. TAO and my application are affected.
>         OTHER (please specify)?
>             -
>
>     SYNOPSIS:
>         There are two issues noticed when fetching IOR via HTTP, by using
> orb->string_to_object() function.
>         Because of these issues this functions fails in fetching IOR, in
> both cases with error.
>
>     DESCRIPTION:
>         First issue is that TAO_HTTP_Reader::receive_reply (void) function
> in HTTP_Handler.cpp
>         expects string "200 OK" as mandatory in HTTP response. However,
> for Ericsson and NSN systems
>         I was connecting with my app, URL for Naming service IOR points to
> a file on web server which
>         contains IOR string. HTTP response in both Ericsson and NSN cases
> contains just IOR string, there
>         are no HTTP response headers. Because of that this function fails
> in reading IOR.
>
>         Second issue is a bug and it's more serious. It is in the same
> function as first issue,
>         TAO_HTTP_Reader::receive_reply (void) in HTTP_Handler.cpp. HTTP
> response data is read and
>         placed into list of "ACE_Message_Block"s, each of MTU size. These
> blocks are after that
>         concatenated into one string in function
> TAO_HTTP_Parser::parse_string in HTTP_Parser.cpp.
>         The problem is that "ACE_Message_Block"s created in
> TAO_HTTP_Reader::receive_reply are NOT
>         zero terminated and TAO_HTTP_Parser::parse_string uses
> ACE_OS::strlen in
> ACE_String_Base<ACE_CHAR_T>::operator+= (const ACE_CHAR_T* s) in
> String_Base.cpp.
>        This operator is used for concatenation of "ACE_Message_Block"s as
> C strings.
>         "strlen" expects each of these blocks to be zero terminated and
> produces unpredictable results
>         with a lot of garbage bytes in resulting IOR string. Luckily,
> while testing the code I've noticed
>         that eventually "strlen" encounters zero byte after at most 50
> bytes, well after the end of
>         ACE_Message_Block, so it does not cause more damage than putting
> garbage into final IOR string.
>
>     REPEAT BY:
>         CORBA::Object_var obj=orb->string_to_object("
> http://xxxxxxxxxxxxxxxxxxxxxxxxxx");
>         URL should be a file on web server containing IOR string and no
> HTTP response headers should be present, this is to reproduce only first
> issue.
>
>         Second issue (bug) is always appearing if there is at least one
> "ACE_Message_Block" containing response which is not zero terminated.
>         Proper zero termination could happen only by chance if the next
> byte after "ACE_Message_Block" is zero or if HTTP response
>         is such that contains zero bytes, which is unlikely.
>
>     SAMPLE FIX/WORKAROUND:
>         Here is a complete modified TAO_HTTP_Reader::receive_reply()
> function with issues corrected.
>
> int
> TAO_HTTP_Reader::receive_reply (void)
> {
>   size_t num_recvd = 0;
>   char buf [MTU+1];
>   char *buf_ptr = 0;
>   size_t bytes_read = 0;
>
>   // Receive the first MTU bytes and strip the header off.
>   // Note that we assume that the header will fit into MTU bytes.
>   if (peer ().recv_n (buf, MTU, 0, &num_recvd) >= 0)
>     {
>       //Make sure that response type is 200 OK
> // It could be "IOR:" also!!!
>       if (ACE_OS::strstr (buf,"200 OK") == 0)
> {
> // If response is pure IOR string it should begin with "IOR:" without any
> white spaces before it
> // This could be modified in future to skip leading white spaces if there
> is a possibility that there are white spaces before "IOR:" string
> if (*buf == 0 || ACE_OS::strstr (buf,"IOR:") != buf) // *buf==0 is here
> because strstr returns buf when buf is empty string
> TAOLIB_ERROR_RETURN ((LM_ERROR,
> "TAO (%P|%t) - HTTP_Reader::receive_reply, Response is not 200 OK nor
> IOR:\n" ), -1);
> }
>
>       // Search for the header termination string "\r\n\r\n", or "\n\n". If
>       // found, move past it to get to the data portion.
>       if ((buf_ptr = ACE_OS::strstr (buf,"\r\n\r\n")) != 0)
>         buf_ptr += 4;
>       else if ((buf_ptr = ACE_OS::strstr (buf, "\n\n")) != 0)     //for
> compatibility with JAWS
>         buf_ptr += 2;
>       else
>         buf_ptr = buf;
>
>       // Determine number of data bytes read. This is equal to the
>       // total bytes read minus number of header bytes.
>       bytes_read = num_recvd - (buf_ptr - buf);
>
>     }
>   else
>     {
>       TAOLIB_ERROR_RETURN ((LM_ERROR,
>                          "TAO (%P|%t) - HTTP_Reader::receive_reply, error
> while reading header\n"), -1);
>     }
>
>   // ***************************************************************
>   // At this point, we have stripped off the header and are ready to
>   // process data. buf_ptr points to the data
>
>   ACE_Message_Block* temp = 0;
>   ACE_Message_Block* curr = this->mb_;
>
>   ACE_NEW_RETURN (temp,
>                   ACE_Message_Block (bytes_read + 1), // Added "+1" to
> make room for byte 0 to make 0 terminated string.
>                   -1);
>   curr->cont (temp);
>   curr = curr->cont ();
>
>   // Copy over all the data bytes into our message buffer.
>   if (curr->copy (buf_ptr, bytes_read) == -1)
>     {
>       TAOLIB_ERROR_RETURN ((LM_ERROR, "TAO (%P|%t) -
> HTTP_Reader::receive_reply, error copying data into Message_Block\n"), -1);
>     }
>
> // 0 - terminate string
> *curr->wr_ptr () = 0;
>   curr->wr_ptr (1);
>
>   // read the rest of the data into a number of ACE_Message_Blocks and
>   // chain them together in a link list fashion
>   num_recvd = 0;
>
>   do
>   {
>     if (curr->space () == 0)
>     {
>       ACE_NEW_RETURN (temp,
>                       ACE_Message_Block (MTU + 1), // Added "+1" to make
> room for byte 0 to make 0 terminated string.
>                       -1);
>       curr->cont (temp);
>       curr = curr->cont ();
>     }
>
>   if (peer ().recv_n (curr->wr_ptr (), curr->space (), 0, &num_recvd) >= 0)
>     {
>       // Move the write pointer
>       curr->wr_ptr (num_recvd);
>
> // 0 - terminate string
> *curr->wr_ptr () = 0;
>       curr->wr_ptr (1);
>
>       // Increment bytes_read
>       bytes_read += num_recvd;
>
>     }
>   else
>     {
>       TAOLIB_ERROR_RETURN ((LM_ERROR,
>                          "TAO (%P|%t) - HTTP_Reader::receive_reply, Error
> while reading header\n"), -1);
>     }
>   } while (num_recvd != 0);
>
>   // Set the byte count to number of bytes received
>   this->bytecount_ = bytes_read;
>
>   return 0;
> }
>
>
> --
>
> Jovan Bunjevački.
>



-- 

Jovan Bunjevački.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://list.isis.vanderbilt.edu/pipermail/tao-bugs/attachments/20160420/44b123a6/attachment-0001.html>


More information about the tao-bugs mailing list