DllStructGetData() truncation

[Valik]

DllStructGetData() truncates the last element of a (char/wchar) array to ensure null termination. The code needs modified to secretly allocate a larger buffer and secretly insert a null terminator outside the user-requested bounds so the user's data is not altered.

This is a display-only issue, the underlying data is not changed.

[Jon]

I'll sort this when I rewrite DllStruct.

[J-Paul Mesnage]

Replying to Valik:

DllStructGetData() truncates the last element of a (char/wchar) array to ensure null termination. The code needs modified to secretly allocate a larger buffer and secretly insert a null terminator outside the user-requested bounds so the user's data is not altered.

This is a display-only issue, the underlying data is not changed.

This will have drawback when the user use DllStructGetptr to access inner structure.
Scripter should respect the +1 if needed and only DllStructSetData will be protected against the overflow as DllCall using this struct can always overflow

[Valik]

Replying to Jpm:

This will have drawback when the user use DllStructGetptr to access inner structure.
Scripter should respect the +1 if needed and only DllStructSetData will be protected against the overflow as DllCall using this struct can always overflow

There is no drawback to allocating a larger buffer than requested. Once this ticket is closed and the thread where it was discovered fades away, user's won't even know that we allocate a larger buffer than they request. If your concern is a 1- or 2-byte buffer overflow won't be detected, then we can change the extra out-of-user-area bytes to read-only and trap any exception that occurs.

[Valik]

... then we can change the extra out-of-user-area bytes to read-only and trap any exception that occurs.

Oh, and we should probably be doing this anyway. I suggested it once. It's a really easy way to catch buffer overflows with DllStruct stuff, so if we are going to allocate larger blocks for "house-keeping" data we should go ahead and mark it read-only and detect an attempts to write to it.

[J-Paul Mesnage]

Replying to Valik:

... then we can change the extra out-of-user-area bytes to read-only and trap any exception that occurs.

Oh, and we should probably be doing this anyway. I suggested it once. It's a really easy way to catch buffer overflows with DllStruct stuff, so if we are going to allocate larger blocks for "house-keeping" data we should go ahead and mark it read-only and detect an attempts to write to it.

I don't understand how we can catch buffer overflow when dllstruct use by a DllCall that can set beyound whatever is hidden allocated

[Valik]

Jon, is this fixed in your DllStruct implementation rewrite?

[Jon]

No. I looked at it briefly, decided it was right as it was and then changed my mind, got confused and ignored it.

[Valik]

Jon, the current behavior is wrong and causes data-loss of 1 character every time a string is read when strlen() == sizeof(buffer). For example, BSTR's don't have null terminators, they store the size in the first WORD (or something like that). Attempting to use this with an exact-sized BSTR-style string would cause data-loss of the last character.

AutoIt is assuming all array's are sized to correctly hold a null terminator and that it's okay to replace the last character with a null terminator when reading. This assumption is wrong since we don't know enough about how the data is being used to make that assumption.

[Valik]

Fixed in version: 3.2.13.6

[Valik]

Re-opening because I didn't fix this correctly the first time around. Also setting as blocking because I kind of made things worse.

[Valik]

Fixed in version: 3.2.13.8