On 17/04/18 10:27, Hans Leidekker wrote:
On Tue, 2018-04-17 at 09:28 -0500, Zebediah Figura wrote:
On 17/04/18 09:17, Hans Leidekker wrote:
On Mon, 2018-04-16 at 20:20 -0500, Zebediah Figura wrote:
+UINT unmarshal_record(const struct wire_record *in, MSIHANDLE *out) +{
- MSIRECORD *rec;
- unsigned int i;
- UINT r;
- rec = MSI_CreateRecord(in->count);
- if (!rec) return ERROR_OUTOFMEMORY;
- for (i = 0; i <= in->count; i++)
- {
switch (in->fields[i].type){case MSIFIELD_NULL:break;case MSIFIELD_INT:case MSIFIELD_INTPTR:r = MSI_RecordSetInteger(rec, i, in->fields[i].u.iVal);break;MSIFIELD_INTPTR is used to store a pointer in MsiViewFetch. We should probably get rid of that but while we have it I think it would be better to print a message here and return an error.
Unless I'm misunderstanding the code, it's necessary for MsiViewModify to work, and I have tests showing that function works across custom actions.
The pointer would get truncated in a 64-bit process if you store it like that. Since this field type is only used to validate the record in MsiViewModify I think it would be better to get rid of it and find some other way to validate the record.
Alright, I'll look into fixing this. Thanks for the review.