flaky regression(?) - patch.py?id=1012510895681951425038278

[Francois Gouget]

On Tue, 5 Feb 2002 lawson_whitney at juno.com wrote:
[...]
> This fixes the problem I reported.  Maybe we should make the filename
> longer to accomodate the trailing \0, maybe because it is a fixed
> length, the trailing \0 is gratuitous, but as it was, that stepped all
> over bigBlockSizeBits, generally disrupted my app, and looks sort of
> like an off by one error.  I'll attach a copy in case your mailer folds
> it.

   I don't know what the code is supposed to do so I cannot comment much
on your patch. But it looks reasonable. But this file contains quite a
few other cases that are definitely not reasonable:

   Context:
struct StorageImpl { ...
    WCHAR filename[PROPERTY_NAME_BUFFER_LEN];
.. };


 * in StorageBaseImpl_RenameElement()
   StorageBaseImpl_CreateStream()
   StorageImpl_CreateStorage()

    renamedProperty.sizeOfNameString =
      ( lstrlenW(pwcsNewName)+1 ) * sizeof(WCHAR);

    if (renamedProperty.sizeOfNameString > PROPERTY_NAME_BUFFER_LEN)
      return STG_E_INVALIDNAME;

    strcpyW(renamedProperty.name, pwcsNewName);

   Should not cause crashes but shouldn't the test be:
renamedProperty.sizeOfNameString > PROPERTY_NAME_BUFFER_LEN*sizeof(WCHAR)

   Although this is a contrieved way to put it.


 * StorageImpl_ReadProperty()

    WCHAR *propName = (index == This->rootPropertySetIndex) ?
	    		This->filename : (WCHAR *)currentProperty+OFFSET_PS_NAME;
    memset(buffer->name, 0, sizeof(buffer->name));
    memcpy(
      buffer->name,
      propName,
      PROPERTY_NAME_BUFFER_LEN );


   - Do we really have a garantee that there will be
     PROPERTY_NAME_BUFFER_LEN bytes available for memcpy to copy?
     (grrr, just saw that OFFSET_PS_NAME==0, still you either suppose
     it's 0 or not)
   - Furthermore since this is bytes, there could be as much as
     2*PROPERTY_NAME_BUFFER_LEN bytes to copy!
   - What's the point of the memset by the way?

  -> a unicode strncpy seems indicated


 * StorageImpl_WriteProperty
   -> pretty much the same problem


--
Francois Gouget         fgouget at free.fr        http://fgouget.free.fr/
           Cahn's Axiom: When all else fails, read the instructions.