:: Re: [DNG] Systemd Shims
Αρχική Σελίδα
Delete this message
Reply to this message
Συντάκτης: Edward Bartolo
Ημερομηνία:  
Προς: dng
Αντικείμενο: Re: [DNG] Systemd Shims
I reorganised the C source code into header files and C code files. I
also tested the backend to make sure the reorganisation of the code
did not impact its functionality. I also included several 'patches' as
suggested and made sure strcat does behave properly.

Needless to state the obvious this code is still in development which
means it is not yet ready to be used. However, for development
evaluation purposes it works.

Here is my humble C code.

-------------------------------------------------------------------------------------
On 20/08/2015, Roger Leigh <rleigh@???> wrote:
> On 20/08/2015 11:27, Rainer Weikusat wrote:
>> Roger Leigh <rleigh@???> writes:
>>> On 19/08/2015 17:39, Rainer Weikusat wrote:
>>
>> [...]
>>
>>>> static void saveFile(char* essid, char* pw) //argv[1], argv[2]
>>>> {
>>>>     char *path;
>>>>     FILE *fp;
>>>>     unsigned p_len, e_len;

>>>>
>>>>     p_len = strlen(IFACES_PATH);
>>>>     e_len = strlen(essid);
>>>>     path = alloca(p_len + e_len + 2);

>>>>     
>>>>     strcpy(path, IFACES_PATH);
>>>>     path[p_len] = '/';
>>>>     strcpy(path + p_len + 1, essid);

>>>>     
>>>>     fp = fopen(path, "ab+");
>>>>     fprintf(fp, IFACE_TMPL, essid, pw);
>>>>     fclose(fp);
>>>> }

>>>>
>>>> int main(int argc, char **argv)
>>>> {
>>>>     saveFile(argv[1], argv[2]);
>>>>     return 0;
>>>> }

>>>
>>> I'm not picking on this post in particular out of the rest of today's
>>> thread, but I did think this was a good example. While I don't want
>>> to act like a rabid C++ zealot, stuff like this really makes me
>>> shudder due to the fragility and unnecessary complexity for something
>>> which is really trivial.
>>>
>>> While the relative safety and security of C string handling can be
>>> debated, I do think the question needs asking: Why not use a language
>>> with proper safe string handling and avoid the issue entirely?
>>> It's only "safe" until it's refactored to break the existing
>>> assumptions and make it accidentally unsafe. The constants such as 2,
>>> 1 plus the strlen() calls are prime candidates for future bugs. It's
>>> not like this /needs/ to be done in C.
>>
>> The 'constant 2' follows from the fact that the length of the result
>> string is the length of the path plus the length of the essid string
>> plus two more bytes/ characters, namely, the '/' and the terminating
>> zero.
>>
>> The 'constant 1', in turn, follows from the fact that the essid has to
>> be appended after the string made up of the path and the added '/'.
>>
>> That's how string processing in C happens to look like because of how
>> the language (library, actually) defines a string and because of the
>> operation supposed to be performed here (combine three different parts,
>> one with constant length).
>>
>> Could you perhaps elaborate on what you were writing about? Minus trying
>> to start a language war, that is. The original author chose C. Because
>> of this, I wrote and posted a simple example how to do string processing
>> in C without relying on 'large enough' fixed size buffers.
>
> The rationale for the use of the constants is fine. But consider that
> the code does not document where those numbers come from, and fact that
> code to calculate the buffer size and the code to copy data into the
> buffer are separate steps. This is where problems can occur. Maybe not
> right now, after all you got it right when you wrote it, one would hope.
> But when it comes to future modifications, you must update all the
> size calculations and constants in line with any changes to how the
> buffer is filled, and this is a prime candidate for mistakes and
> consequent crashes and/or buffer overflows. When it comes to making
> modifications, you or whoever is making the change, needs to work out
> exactly what the intent of the orginal code was--i.e. re-derive all the
> constants and re-compute them correctly to match the new behaviour.
> This is often non-trivial depending on the nature of the string
> manipulation.
>
> The fact that C code is a continual source of such quality and security
> defects is a clear indication that in general people *don't* get this
> right even when they think they are geniuses who know their own code.
> This example is short, but people continue to routinely screw up even
> stuff this simple, and it only becomes more likely with more complexity.
>
> IMO, stuff like this just doesn't belong in any program that claims to
> be secure. Especially in one that's setuid root doing privileged
> operations.
>
> When I wrote the schroot(1) tool, which is setuid-root out of necessity
> since chroot(2) is privileged, I originally wrote it in C, but converted
> it to C++ primarily due to the security considerations, of which
> avoiding insecure and buggy string handling was the prime consideration.
> It removes this entire class of bugs and security exploits at a
> stroke. And if I .reserve() space in a string for efficiency, it's as
> efficient as all the manual C manipulations, but safe from overflow, and
> if I get it wrong it'll merely perform another allocation rather than
> being exploitable. i.e. it's as efficient as string manipulation in C,
> only vastly less complex and with safety factored in.
>
> All too often the attitude of C programmers is "it'll be perfect and
> efficient since I won't screw up". But we all screw up at some point.
> And the question then is, "what are the consequences of screwing up".
> And here, the consequences are severe. But with std::string, are
> entirely avoidable. So we both reduce the chance of a screwup by making
> the API match our intent exactly without lots of extra make-work to
> allocate buffers and unsafely mangle strings, and we also mitigate for
> any mistakes in calculations by the failure case being a slightly less
> efficient allocation strategy rather than a root exploit.
>
>
> Regards,
> Roger
> _______________________________________________
> Dng mailing list
> Dng@???
> https://mailinglists.dyne.org/cgi-bin/mailman/listinfo/dng
>