Uploaded image for project: 'Firestorm'
  1. Firestorm
  2. FIRE-20094

[PATCH] flickr uploader should check std:string.max_size() when before appending image data to avoid buffer overrun

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: Phoenix Firestorm 4.7.9
    • Fix Version/s: Phoenix Firestorm 4.7.10
    • Component/s: None
    • Labels:
    • Environment:
    • SL Avatar Name:
      Beq Janus
    • Patch attached:
      Patch attached
    • Reported In:
      Firestorm 4.7.9.50527 Release

      Description

      It may be that this is impossible to hit due to limits enforced elsewhere however, in the exoflickr.cpp module the upload function ( void exoFlickr::uploadPhoto(const LLSD& args, LLImageFormatted *image, response_callback_t callback) ) builds a stream to be sent to flickr as a POST request. This is then copied to a std::string and appended to. The append call pulls the image data from the llImage without checking that there is appropriate capacity and could in theory fail.

      std::string post_str = post_stream.str();
      std::string post_tail = "\r\n-" + boundary + "-";

      std::string post_data = post_str;
      post_data.append( reinterpret_cast< char const* >( image->getData() ), image->getDataSize() );
      post_data.append( post_tail.c_str(), post_tail.size() );

      The following code should be testing the max_size of the implementations std::string and adjusting for the remaining capacity before comparing against the image payload size.

      // check that we have enough capacity in the string to append the data
      int image_data_len = image->getDataSize();
      if(image_data_len< post_data.max_size() - post_data.size() - post_tail.size())

      { post_data.append(reinterpret_cast< char const * >( image->getData()), image->getDataSize()); post_data.append(post_tail.c_str(), post_tail.size()); }

      else

      { // Not a very satisfactory way to handle this. LLWARNS("FlickrAPI") << "Image data too large. Malformed post body produced." << LL_ENDL; }

      Please note that the error handling here is pretty poor but there does not seem to be an obvious alternative than to allow the corrupt buffer to attempt upload.

      Attached patch but please note I am not currently able to build so this has not been tested or even compiled.

      regards

      Beq x

        Activity

        Hide
        beqphoenixjira Beq Janus added a comment - - edited

        I'm an idiot...this module is no longer used I think. There is better code checking this in the llflickrconnect module, where the body data is appended to the buffer directly

        Update: I spoke with Ansa and this module is still used for opensim so the issue remains valid. Ansa also gave some feedback on improving the error handling.

        Show
        beqphoenixjira Beq Janus added a comment - - edited I'm an idiot...this module is no longer used I think. There is better code checking this in the llflickrconnect module, where the body data is appended to the buffer directly Update: I spoke with Ansa and this module is still used for opensim so the issue remains valid. Ansa also gave some feedback on improving the error handling.
        Hide
        ansariel.hiller Ansariel Hiller added a comment -

        Pushed the patch with the modification discussed inworld and a fix for a small off-by-one fault in the if-condition in 50930 (http://hg.phoenixviewer.com/phoenix-firestorm-lgpl/rev/a4663798b02e).

        Show
        ansariel.hiller Ansariel Hiller added a comment - Pushed the patch with the modification discussed inworld and a fix for a small off-by-one fault in the if-condition in 50930 ( http://hg.phoenixviewer.com/phoenix-firestorm-lgpl/rev/a4663798b02e ).

          People

          • Assignee:
            ansariel.hiller Ansariel Hiller
            Reporter:
            beqphoenixjira Beq Janus
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: