The reason is quite simple: your use of strcat. strcat searches for the end of the string and adds the characters there. The end of the string is marked by '\0', so if you don't initialize the buffer to all 0 it will search past the end of the buffer and then try to write there, resulting in the access violation.

But I don't see why you couldn't use strcpy instead of strcat here.

About the inefficiency: first, is there any specific reason why you're keeping track of the buttons yourself instead of letting the message loop handle it?

Second: instead of one for-loop and several ifs use several for-loops. This removes many unnecessary - because impossible - if-statements.