sscanf_s stack corruption

[UserLoser]

Code Select Expand


typedef struct socialsecuritynumber_t {
unsigned short Area;
unsigned short Group;
unsigned short Serial;
} SOCIALSECURITYNUMBER;

void ProcessEmployees(ifstream &InputFile)
{
char FileData[64], Exception[64];
char FirstName[32], LastName[32];
float PayRate = 0;
SOCIALSECURITYNUMBER SSN;

// Loop through entire file
for(int LineNumber = 1; !InputFile.eof(); LineNumber++) {
// Get line
InputFile.getline(FileData, sizeof(FileData));

// Get our data
if(sscanf_s(FileData, "%u-%u-%u %s %s %f", &SSN.Area, &SSN.Group, &SSN.Serial, FirstName, sizeof(FirstName), LastName, sizeof(LastName), &PayRate) != 6) {
sprintf_s(Exception, sizeof(Exception), "Invalid entry on line #%i\n", LineNumber);
throw Exception;
}
}

// Close the file
InputFile.close();
InputFile.clear();
}


Sample input:

Code Select Expand


331-92-1974 Michael Jordan 22.94


Why stack corruption?  I never use this function, this would be why, but I have to for this particular thing.

[Kp]

Assuming it behaves like sscanf (since I have no man page for sscanf_s), your problem is the types in SOCIALSECURITYNUMBER.  %u calls for an unsigned int, which is usually 32 bits.  You're allocating 16.  Try %hu or up the size of the elements to an unsigned int.

[UserLoser]

Oh ok that works.  Thank you Kp.  Never knew that the size really mattered for this

[warz]

Size does matter.

[MyndFyre]

Size does matter.

That's not what she said. 

[Kp]

Oh ok that works.  Thank you Kp.  Never knew that the size really mattered for this

For many things, it won't.  However, scanf and friends are varargs and thus no type checking is performed on the trailing arguments.  You could even pass non-pointers, which would have really dire consequences.  The argument would be interpreted as a pointer and scanf would write to an undesirable location, hopefully causing an immediate crash.

If you had gone the other way (using unsigned int but specifying %hu), you would have been fine on a little endian system, since the low word of a 32bit value is overlaid with the entirety of the 16bit value.  On a big endian system, it would have failed in a more obvious way (the low 16 bits remain zero, the high 16 bits get loaded with the data you're trying to convert).

To further complicate matters, you could have safely used %u in a printf statement and passed an unsigned short.  The short would be promoted to a full int for stack alignment purposes, making the %u correct.