[blauditore]

I'm curious, wouldn't this also be caught by static code analysis tools, at least today? An assigment inside an if condition is both, most likely a mistake, and fairly easy to detect automatically.

[rocqua]

I would guess this is part of the reason why most modern compilers will indeed emit a warning about assignment within if, for, and while - branch checks.

At the same time, the standard implementation of strcpy is:

    while((*dst++ = *src++));

which has a legitimate reason for doing assignment inside the while condition. Then again, one could argue that the above code is 'too clever'. And I would probably agree.

[ancgop]

However they do not emit a warning if the assignment is parenthesized, like in the exploit. I think static analysis tools are the same, they would be way too chatty if they emitted warning for a parenthesized assignment.

Static analysis already has way too many false positives as it stands. For a well maintained code base the rate can easily be 100% false positives, which gets annoying after some time.

[asddubs]

could do this instead, right?

    do {
       *dst = *src;
       *dst++;
       *src++;
    } while(*dst);

[josefx]

I think you are not copying the terminating nul character.

[FartyMcFarter]

Unless the first character was null, in which case it would be ignored by the condition... Also, you don't need to dereference a pointer in order to increase it.

The grandparent post's code is just nonsensical.

[asddubs]

haha, I was genuinely asking if it made sense, I don't usually do C. Maybe it helped illustrate that the code is too clever for me, at least.

e: I submit my revised code:

    do {
       *dst = *src;
       src++;
       dst++;
    } while(*(dst - 1));

[asveikau]

Looking at src[-1] would work but feels like poor form. My advice is stop trying to make do ... while work here, it isn't looking good.

I feel like more idiomatic iterating through a string tends to loop on src not being a terminator.

    while (*src)
    {
       *dst = *src;
       ++src;
       ++dst;
    }

    *dst = '\0';

I feel like this is idiomatic C but needlessly verbose. Most people would combine the increment with the assignment. And most people would recognize putting it in the while condition as a common strcpy.

[lqet]

I think this is why there are parantheses around current->uid = 0. gcc has the option -Wparentheses, which gives a warning if you write something like this:

  if (a = b) doSomething;

But there is no warning if you write it like this:

  if ((a = b)) doSomething;

The convention is that with these unneeded parantheses, you are signalling that you actually want the assignment here. I would assume other static code analysis tools use this convention as well.