C/C++ strcpy unhandled read violation

Question

unsigned char* Data::getAddress(unsigned char* address)
{
    strcpy((char*)address, (char*)this->_address);
    return (unsigned char*)address;
}

int main()
{
    Data d;
    d.makealinkedlisthere();
    while (d)
    {
       unsigned char address[256];
       printf("0x%08x \r\n",d.getAddress(address));
       d = d.getNext();
    }
    return 0;
}

It returns the first two (which is the same, and it should be different [can tell from the debugger]...) then crashes out.

It just makes a linked list. protected member Data* _next ... a chain of them.

The unsigned char* is from Windows function VirtualQueryEx part of the MEMORY_BASIC_INFORMATION data structure it returns.

this->_address = (unsigned char*)meminfo->BaseAddress; // casted from void*

It is void*, but I see it converted to unsigned char* in other's codes. In the debugger I can see it represented as a hex number.

D1: +    _address   0x7ffd5000 <Bad Ptr>    unsigned char * 
D1->_next:+  _address   0x7f6f0000 "áå•ú`©" unsigned char * 
D1->_next->_next+    _address   0x7ffb0000 " "  unsigned char *

Please show the source code of the makealinkedlisthere() function — Flot2011, Apr 15 '12 at 18:57
You have not provided enough information. What does `Data::makealinkedlistthere` do? — Nicol Bolas, Apr 15 '12 at 18:58
You need to provide more information about Data for us to figure out what is going on here. In particular `makealinkedlisthere` and `getNext`. btw the loop looks very suspicious. — quamrana, Apr 15 '12 at 18:58
You always pass the same pointer into the 'getAddress' method, so it naturally always returns the same result: a pointer to the first character of the 'address' array. It fills that array with different data every time, but your code does not print that data, only a pointer to it. — Koen Van Damme, Apr 15 '12 at 19:00
It just makes a linked list. protected member Data* _next... a chain of them. The unsigned char* is from Windows function VirtualQueryEx part of the MEMORY_BASIC_INFORMATION. this->_address = (unsigned char*)meminfo->BaseAddress;. Put it in the main question to be more clear — user1334943, Apr 15 '12 at 19:01
We mean that you should edit the question in order to provide more information. — quamrana, Apr 15 '12 at 19:02
But `MEMORY_BASIC_INFORMATION.BaseAddress` is not a string. You can't copy it like a string! — Mr Lister, Apr 15 '12 at 19:06
It is void*, but I see it converted to unsigned char* in other's codes. In the debugger I can see it represented as a hex number. D1: + _address 0x7ffd5000 unsigned char * D1->_next:+ _address 0x7f6f0000 "áå•ú`©" unsigned char * D1->_next->_next+ _address 0x7ffb0000 " " unsigned char *... Added this to the above in better readable format. — user1334943, Apr 15 '12 at 19:08
No, the fact that you can cast it to a `char*` doesn't mean you can treat it like a string! — Mr Lister, Apr 15 '12 at 19:27

Abhijit · Accepted Answer · 2012-04-15T19:37:18.590

MEMORY_BASIC_INFORMATION.Base Address is the location of the region of pages and not a string and there is no gurantee that it would be null terminated. From the MSDN site you can see the MEMORY_BASIC_INFORMATION structure

typedef struct _MEMORY_BASIC_INFORMATION {
  PVOID  BaseAddress;
  PVOID  AllocationBase;
  DWORD  AllocationProtect;
  SIZE_T RegionSize;
  DWORD  State;
  DWORD  Protect;
  DWORD  Type;
} MEMORY_BASIC_INFORMATION, *PMEMORY_BASIC_INFORMATION;

To copy the data, you should use a memcpy with size = 255`.

Looking back to your code, provided there is no other issues, changing it to

PVOID Data::getAddress(PVOID address,size) {    
    memcpy((address, (void *)this->_address, size);
    address[size]=NULL;
    return address;
}
int main() {
    Data d;
    d.makealinkedlisthere();
    while (d) {
       unsigned char address[256];
       printf("Address: 0x%08x \n",d.getAddress((PVOID)address),sizeof(address));
       printf("Data: %s\n",(LPSTR)d.getAddress((PVOID)address),sizeof(address));
       d = d.getNext();
    }
    return 0;
}

should work

Please also note, it would be safe to store RegionSize and to do a boundary check before memcpy.

The OP uses only BaseAddress actually, not the whole MEMORY_BASIC_INFORMATION struct. So `_address` should be a PVOID and it can be copied by simple assignment. — Mr Lister, Apr 15 '12 at 19:26

C/C++ strcpy unhandled read violation

1 Answers1