CodeSOD: A Big Raise

Everyone likes getting a pay raise. Well, I suppose it depends on why. And HR isn't going to be too happy about your raise if it comes as the result of an easy-to-avoid software bug.

Cedric V's company makes payroll software mostly used in and around France. One of their customers had a problem: when paying employees, it would give them a significant raise- sometimes by two orders of magnitude, rarely by three or four.

What was surprising is that this happened to one customer, but none of their other customers. They couldn't replicate it on their test environment either. But once Cedric started digging into the code, it wasn't hard to understand what the root cause was.

double origin = GetDoubleFromRandomMethod();string str = origin.ToString().Replace(".",",");decimal result = decimal.Parse(str);

The good thing I can say about this C# code is that it does, eventually, handle currency values using decimal, which guarantees that we won't lose precision. Of course, that decimal derives from a double, so we might already have errors.

Those errors, of course, don't matter, because somebody made some terrible assumptions about locale and added in an unnecessary ToString operation with an equally unnecessary Replace.

The ToString method on a double, by default, uses format "G" (for "general"), which is locale specific. So, if you're in a French locale, it will use the "," as a decimal separator. "3.14" in a US locale would be "3,14" in a French locale.

So, if this is used in a French locale, the Replace does nothing, and the Parse successfully converts the value back into a decimal.

But the customer with the sudden raise problem didn't run their servers configured in a French locale. Since much of their business involved US customers, they ran their servers in a US locale. In that case, the Replace would change the "." to a ",". The Parse would interpret the comma as a "thousands separator", but the parser just ignores that symbol, even if it's in the wrong place. So "3.14" becomes "3,14", which gets parsed as "314". The number of significant figures the double has behind the decimal place determines how many orders of magnitude someone gets a raise by.

I suppose they should be happy that there values coming out of that GetDoubleFromRandomMethod aren't the result of any division operations. One 8/12 and you are paying out a satanic amount in quadrilions.

[Advertisement] Otter - Provision your servers automatically without ever needing to log-in to a command prompt. Get started today!