[1/3] dpnet: Add check for mismatched string lengths (try 3)

[Stefan Dösinger]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi,

One more thing:
> +        size = sizeof(localhost); +        hr =
> IDirectPlay8Address_GetComponentByName(localaddr,
DPNA_KEY_HOSTNAME, buffer, &size, &type);
Please use size = sizeof(buffer) here. Buffer is the variable you pass
to GetComponentByName.

Cheers,
Stefan

Am 2015-03-04 um 17:23 schrieb Stefan Dösinger:
> Hi,
> 
> Am 2015-03-04 um 04:15 schrieb Alistair Leslie-Hughes:
>> +        hr = IDirectPlay8Address_AddComponent(localaddr,
>> DPNA_KEY_HOSTNAME, "testing", 8, DPNA_DATATYPE_STRING_ANSI); +
>> ok(hr == S_OK, "got 0x%08x\n", hr);
> This still uses a hardcoded size. I recommend to add an array
> const char test_str[] = "testing" similar to the localhost WCHAR.
> That way you can use the variable in all places.
> 
>> +            if (((strlenW((WCHAR*)lpvData)+1)*sizeof(WCHAR)) !=
>> dwDataSize) +            { +                WARN("Invalid STRING
>> size, returning DPNERR_INVALIDPARAM\n"); +                return
>> DPNERR_INVALIDPARAM; +            }
> There is a theoretical issue here that I don't know if we should
> care about. Assume e.g. dwDataSize is 10, but lpvData points to a
> blob that doesn't have a NULL byte before the end of the last
> defined page. In this case strlen will search for a string
> termination and segfault. strnlen could abort and return an error
> instead of segfault.
> 
> I did some quick testing on Windows and it seems that this
> function uses a try / catch block to return gracefully in case of
> an invalid pointer. Generally we don't add tests that cause
> exceptions and I don't think we should test this here. And I have
> found no case where strnlen is used in Wine, and we have no
> strnlenW, so I don't think we should use it here. If I pass an
> invalid non-null pointer I get 0x80004003. If I pass a length = 0 I
> get DPNERR_INVALIDSTRING.
> 
> Also checking !((char *)lpvData)[dwDataSize - 1] would be wrong, 
> windows gracefully returns DPNERR_INVALIDPARAM on lpvData="123", 
> size=999999.
> 
>> +        hr = IDirectPlay8Address_AddComponent(localaddr,
>> DPNA_KEY_HOSTNAME, "testing", sizeof("Testing")+2,
>> DPNA_DATATYPE_STRING_ANSI); +        ok(hr ==
>> DPNERR_INVALIDPARAM, "got 0x%08x\n", hr);
> 
> Really minor issue: sizeof("testing") vs sizeof("Testing"). The
> result is the same, but a stupid compiler might place both strings
> in the executable. The variable I suggested above would avoid this
> issue here as well.
> 
> 

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBAgAGBQJU9zQxAAoJEN0/YqbEcdMwQFUP/ApILUlGHoweVmzD8a9gzDnp
wyEZiJV50LwbAUMaFG7BeZjvDfKGxPD8cDNJpHYoA8CBYg8knv6d/7QR8qab3pg7
zGAtX/82S2P0RnBxl9YrY4ruMUvXaej2sWJnxJkchffP1iWQ0ioKzPhTh7aHZnZH
ZKey82eyZ3vk3COzVIoVV+Qpw7WDqFcVOm7//s2RxOiTFntM+IzD1kTWWZ3WrJxh
8fsI+7/tPCxsy7/ayotttFAoge7e9awOy7U07C1R9vhc2dNgAY0qy+S8ZBZ1OnLE
EKjNxM2SGImtdEyPyXoCM9TfS9PtlnABKNSDyw9L6544LznIWf9u2QVkmXbMM+EP
cof3TFolRjKwq1M2WFHlnQxDb1CE9gfmLyIps2NenjOxzlC2efp/PnvZK3lrhhNi
F/Vi1T6QM4JcREo++oyb//bEUqefq4v3sSzxfq0f1vEgd0Xa3aoRogocaCgsGtaL
uRrkqWyt2HLtvsuawOg3PxiP7UQKrqmrKjKuw/UxPBU+7ATgNOLhIkmyQqDIdCgD
pOOxErtwRxsSA1kYTphlcVkKLi6sJq0V2GXV303Y+SBvqS0g91xm3ejk8MCL+X7z
Lt+4tIzkvFttapDvmgsCaRhfOQ2xriLE6stP5OWRmzionQu3l1vMhR0ZP/ADHal8
Y7rB5wrSwFmfF+i7m5Gl
=oWSc
-----END PGP SIGNATURE-----