Valhalla Legends Forums Archive | C/C++ Programming | sscanf_s stack corruption

AuthorMessageTime
UserLoser
[code]
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();
}
[/code]

Sample input:
[code]
331-92-1974 Michael Jordan 22.94
[/code]

Why stack corruption?  I never use this function, this would be why, but I have to for this particular thing.
November 21, 2006, 1:27 AM
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.
November 21, 2006, 1:32 AM
UserLoser
Oh ok that works.  Thank you Kp.  Never knew that the size really mattered for this
November 21, 2006, 1:39 AM
warz
Size does matter. :P
November 21, 2006, 1:49 AM
Myndfyr
[quote author=warz link=topic=16059.msg161476#msg161476 date=1164073799]
Size does matter. :P
[/quote]
That's not what she said.  :P
November 21, 2006, 2:06 AM
Kp
[quote author=UserLoser link=topic=16059.msg161474#msg161474 date=1164073192]
Oh ok that works.  Thank you Kp.  Never knew that the size really mattered for this
[/quote]

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.
November 21, 2006, 2:31 AM

Search