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

[Francois Gouget]

On Tue, 5 Feb 2002 [e-mail] 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         [e-mail]        http://fgouget.free.fr/
           Cahn's Axiom: When all else fails, read the instructions.