Foot, meet stack

tarunik

Somewhat anonymized, but still terrible no matter how you slice it:

        SomeLongIntTypedef value;
        _some_ui_field.get_val(value);
        char buf[2];
        // Casting is needed because SomeLongIntTypedef is defined as long int
        sprintf(buf, "%02d", static_cast<int>(value));

Keep in mind that _some_ui_field is a custom class that sits atop of several layers of code to get to the actual UI widget the value comes from, and the UI widget itself unconditionally hands back strings.

chubertdev

obj.get_val(value) is a huge WTF by itself.

chubertdev

Seriously, get the value of value? value itself is not a value, you have to get the value of value.

tarunik

@chubertdev said:

Seriously, get the value of value? value itself is not a value, you have to get the value of value.

It's an out parameter, which makes it more of a WTF, even.

chubertdev

That's not too uncommon, depends on the usage.

dkf

@tarunik said:

char buf[2];
sprintf(buf, "%02d", static_cast<int>(value));

Oh boy! Writing three bytes into a two-byte buffer. If that's currently working, it's by sheer luck.

I remember having a very similar problem in some code I was porting years ago (from big-endian Solaris/SPARC to little-endian Linux/x86) where the terminating zero ended up overwriting part of the stack frame metadata and crashing both the process and the debugger I was trying to use to diagnose the problem.

EvanED

@dkf said:

Oh boy! Writing three bytes into a two-byte buffer. If that's currently working, it's by sheer luck.

Not that you're wrong, and that is an abomination, but if there are no other char variables in that function I'd actually expect it to work reliably on typical platforms, which will align things to give padding. I'm not sure if I'd call that quite "sheer luck."

dkf

@EvanED said:

Not that you're wrong, and that is an abomination, but if there are no other char variables in that function I'd actually expect it to work reliably on typical platforms, which will align things to give padding. I'm not sure if I'd call that quite "sheer luck."

It might be possible to predict that the code will execute safely on a particular architecture, but nobody's actually promised anything of the sort. (It depends on which end of the word is holding that pair of characters and what exactly comes next. It really isn't guaranteed to be a gap for alignment; that could be at the other end of the array.) It is Undefined Behaviour, indeed, a true clbuttic example of it, and can sometimes go catastrophically wrong.

Once bitten by this, you get very cautious.

tarunik

@dkf said:

It might be possible to predict that the code will execute safely on a particular architecture, but nobody's actually promised anything of the sort. (It depends on which end of the word is holding that pair of characters and what exactly comes next. It really isn't guaranteed to be a gap for alignment; that could be at the other end of the array.) It is Undefined Behaviour, indeed, a true clbuttic example of it, and can sometimes go catastrophically wrong.

Once bitten by this, you get very cautious.

Indeed, once bitten, twice quite shy. The folks who brought you this CodeSOD, though...

Suffice it to say that they have no qualms about doing this in other places as well. More examples are on their way here. They also actually wonder why their software cores...

VinDuv

During an internship I had to take some C programs which were running on a Linux distribution released in 2002, and make them work an a more recent (~2010) distribution.
My mentor thought they would Just Work^TM after a quick recompile, because they had never crashed on the previous system. Well, I compiled the first one (in fairness, it didn’t generate too many warnings), and... it crashed right away.
I eventually found (after some tedious debugging) a handful of stack buffer overflows which were previously harmless but were now corrupting pointers.