Can you trust your security application?

Let me begin with a nice mathematical problem.

Say you have a divorced couple that are currently discussing over the phone how to divide up their possessions. They pretty much split everything equally, till they reach their car. They know they won't get nearly as much as it's worth by selling it, so they'd prefer if one of them would just keep it, but they can't agree to which one. They don't want to meet and they don't want to get another party involved. How can they determine which one should get the car?

They can flip a coin on it, but since this is over the phone, both parties won't be able to see it and agree. Ideally one side would decide what heads and tails means, and the other would say if it is heads or tails, but they need a secure method to transfer these secrets to each other, without either end fearing of some sort of compromise.

This problem is what today's public/private key secure communication technology is based upon, and we have come up with a solution.

One party would take two very large prime numbers, and multiply them and transfer this number to the other party. The first party keeps the two factors as a secret, the other party has no reasonable way to determine the two factors without months of computation. The first party then informs the second party that once the coin is flipped, they will take the larger factor, and look at it's fifth digit from a particular end. If that digit is odd it means the first party chose heads and the other tails, and if it is even, the winning conditions will reverse. Now the second party can flip the coin and report the status back to the first party.

At this point, everything needed to determine the winner has been transferred securely, now the secret can be transferred to decode who the winner was. The first party will tell the second what the two factors were, and they will have reached an agreement on who will retain the car.

Now provided that the duration of this transaction (phone call) was shorter than the time it would take for the second party to factor, and everything was done in the proper order, this entire exchange should be 100% secure. However, what if the second party happened to know that number already, and had the factors on hand? That party could cheat, and tamper with the exchange of information, rendering the transaction insecure.

Now when we look at secure communication technology today, they generally have each side come up with their own variation of the prime number examples above, and each side sends the other a "public key" to match with their own secret "private key". The private key can't be easily derived from the public key, but one will decode data encrypted with the other. Then when transferring, each side encodes and decodes data with their private and the other's public keys. An attacker can't jump in the middle, because it would need to get the private key which is never distributed in order to decode or impersonate one of them.

This security falls apart when the keys in use are older than their cryptographically secure time frame, or when the application doesn't follow the proper procedures. If keys are long and strong enough, and always replaced before their safe time frame limit approaches, and connections aren't opened indefinitely, one should be safe in relying upon the keys. However, a chain is only as strong as it's weakest link. If the application doesn't follow the proper procedures for key exchange, or has errors in its authentication and validation routines, it could be leaving it's users and their data compromisable.

If your security application is closed source, there is the chance that there is a backdoor programmed into it that will go unnoticed. When a company has few employees in comparison to the sheer amount of code it has in its products, there is little to stop one rouge employee to stick a tiny bit of code into a complex bit of security related code. When the amount of code greatly outnumbers the coders, very few people will bother to look and try to envision all the circumstances a bit of tricky code will have to handle and how it will end up handling it.

Consider how someone hacked into the Linux server and tried to embed a backdoor into the source that would later make up Linux releases around the world, read about it here. They inserted a bit of code that if two rare options were used together, an unprivileged user would gain administrator capabilities on that machine. Such a thing would go unnoticed if the person had the ability to modify the source at will without raising any eyebrows.

Let me offer an example. The SSH program allows a user to connect to a computer running an SSH server. When one wishes to connect to the other, they have to supply a user name and password, as well as tell SSH which modes to use.

Here are two existing options:

-1 Forces ssh to try protocol version 1 only.
-2 Forces ssh to try protocol version 2 only.

No person would normally consider to force SSH to try everything it was going to try anyway. Imagine if the developer of an SSH server stuck in a bit of code that if the client only wanted protocol version 1 and only wanted protocol version 2, to grant access even if the password was incorrect. This rouge developer could then gain access to every machine running his SSH server, and no one the wiser. Once this developer knows few at his company will see his work, he has nothing to lose to add such code. If he happens to be caught, and if the code was in a confusing section, he can say he was trying to handle an invalid case, and apparently didn't do it right, it was only an accident.

When your security program is closed source, do you really want to lay the security of your data in the hands of a disgruntled employee somewhere? Can you really trust the protection of walls that you can't see and has no outside review process? Many internal reviews miss things in tricky sections, especially when the group in question takes pride in their work. I'm reviewing code, and thinking to myself, hey we wrote this, we're world class programmers, we're good at what we do, this code is too tricky to really sit down with a fine tooth comb, I'm sure it's right.

Keep in mind though, just because something is open source, doesn't mean something can't be snuck in either. It's just less likely for that malicious code to remain there for too long if the application/library in question is popular and has enough people reviewing it.

But despite something being open source, malicious code can be snuck in without anyone ever noticing, read this paper for background. Now while the attack described where one edits the compiler to recognize certain security code and handle it in a malicious manner is a bit far fetched, similar attacks a bit closer to home are quite possible. If you're using a Linux distro which offers binary packages, what really stops a package maintainer from compiling a modified application and putting that in the distro's repositories? Those running a secure environment may want to consider compiling certain packages themselves and not trusting binaries that we really have no clue what is in them.

But based on this paper, do we have to worry that the compiler or the OS or other libraries would produce the proper binary when we compile this security application ourselves?

Lucky for us, despite what newbie programmers want, our programming languages aren't made up of a series of very high level functions such as compile(), handle_security(), and the like. Such would make it much easier for someone to make the compiler or the library do something malicious when it encounters such a request in the source. In order for such an attack to be really successful, it would have to understand every bit of code it's compiling to make sure the resulting program won't be able to detect the trojan, which is extremely tricky if not near impossible for a compiler to do. Not using a high level compiler or virtual machine gives us a layer of security in that it would be harder for one to pass out an "evil compiler" that would understand what the developer was trying to do and instead have it do something malicious.

But if such an attack were to take place, we'd have to pull out hex editors and disassemblers to see that such code has been snuck in (something which we must do with closed source applications). Take this a step further, it is theoretically possible if the OS were affected, or if the compiler was so smart that it intimately understood that it was compiling a hex editor or disassembler and the like to stick in code that would subvert file reading on executables and libraries to mask such malicious code even in the binary.

Now while some clever guy out there is thinking to himself: "Oh I'm going to do that, it'll be completely undetectable", such a course of action is much much easier said than done. I would be amazed if even a whole team of programmers would be able to pull such a monumental task off. I wouldn't worry too much that such magical wool has been invented to pull over our eyes when we try to decode a binary in a safe environment.

But I would worry if the application or the libraries it depends on are closed source. And even if we have the source, I would question where the binary we happen to be using comes from. If you're using even an open sourced application in something critical, I would advise to have your binary for your application and related libraries examined in a safe environment just to be sure. I just hope no one subverted an OS out there to alter non executable reading and writing on executable files, and have the OS strip/readd code when executable files are transfered.

File Dialogs - Take 2

My previous article on file dialogs generated much feedback, and I got varied responses from all kinds of people. I'll go over the feedback I got, more data I've received, and what ramifications the last discussion produced.

In my previous article, I didn't discuss Windows Vista at all, as I don't have a copy of it, however several people contacted me with screenshots, and described the system a bit.

Lets take a first look:


There is a lot going on here. Up on top, we have a crumbs based directory browser stolen out of GTK, but of course this dialog is better than what GTK offers. It also provides a refresh button, and has a recent directory drop down. You also get a back and forward button to jump all over when looking for something. A nice addition though is a search box. Not sure where the file is? Then search for it! A nice new intuitive feature (taken from Mac OS X though).

Below this we have options to change what's shown, and the style it's presented in. The new directory button is also plainly visible. Then on the left, we have a quick location list like former versions of Windows had, but now in Windows 6, you can add and delete them to your heart's content. I'm not sure if you can rename them though, readers please write in regarding this. We then have the standard files listing from Windows 4+, with the ability to change the view like we expected. And of course to round it off nicely, we have the file input box to jump to files names quickly, and of course type in a path to move to like us power users want. File management features are also available.

But wait, we're not done yet, check this out:

As you can see, the "Folders" section on the left can be expanded to offer a tree view to browse your system. This borrows on the directory only browser (along side a file browser) from Windows 3, but offered in a more robust tree view. It seems a bit weird to see directories in both the directory and file browsers, but this should keep everyone happy. Many people were annoyed with Microsoft for combining the two in Windows 4+, as it was harder to navigate directories, and had to jump past directories to find files.

It seems like with this new version, Microsoft is trying to please everyone, offering every type of browsing possible, and I applaud them for that. I'd be interested to know if you can turn off the directory display in the main file list pane. If anyone knows, please write in.

I'd like to personally play with this to see how it stacks up against KDE 3.5's file dialog, but this looks really solid. The only problem seems to be they stills stuck with some of their virtual directory nonsense, such that you'll see Desktop/User and Desktop/Documents, when the actual tree is Users/User/Desktop and Users/User/Documents. Guess we can't have everything.

Next up, we'll be revisiting GTK. All the responses except for one to my last article agreed with me as to how bad GTK was. Some even wrote in offering demonstrations showing how it was worse than even I knew.

The one person who wrote in disagreeing offered some interesting data. No, he wasn't a developer telling me GNOME/GTK folks were improving it, and he didn't actually disagree with what I described as being bad. He wrote in to say that he has a completely different dialog!

Let us look at our first screenshot:


As you can see, a location bar is provided along with everything else we were familiar with, so one can quick jump, and this happens to work well. The quick locations on the left are also combined into one, so you can add and remove even the built in ones. Not sure about renaming though. But wait there's more!


As the above shows, it also has sane auto complete, instead of an auto complete where you write /usr and end up with /usr/src. I asked for the source of these changes, if perhaps it was from a new or in development version of GTK or GNOME. I was told that he had these dialogs since he setup his PC years ago, and that it was from a usability patch that he had installed. Unfortunately though, he wasn't sure where he got them from, so I guess I'm still stuck trying to replace FireFox and GAIM on my machine.

Let us take a moment to ponder though that there are usability patches out there to vastly improve GTK/GNOME, but we still have no hint of them making their way into the official versions. Perhaps if we start boycotting GTK apps, we'll see the developers do something sane for once. It'd also be nice if it wasn't as slow as molasses.

Next, we come to the Qt file open dialog. Last time, I showed a preview of what Qt 4.3 was going to offer. It seems I got no limit to the responses thanking me for alerting them to the impending disaster.

A friend of mine who has a neat app he wrote using GTK told me how he recently added file browsing support and was very annoyed at how he had to spend a lot of time writing a new file open dialog from scratch because of how utterly atrocious the built in one was. He told me he was considering switching over to Qt because he heard how superior it is, and how he won't have to put up with such stupidity as it has sane stuff built in. However when he saw what Qt 4.3 was planning, he promptly dropped any considerations he had, as he didn't feel like he needed to switch to a GTK knock off and reimplement the file open dialog again. Let us remember that GTK originally ripped Qt off and we don't need to go flip the tables, and pay attention to the $0.02 we get from developers who can't even figure out how to write a sane file dialog.

Another good friend of mine also took it upon himself to spread the word as much as possible. He mentioned it in #qt on Freenode, an IRC channel with many Qt developers. I'm told they were furious when they saw what changes were being planned.

Apparently all this criticism made its way back to Trolltech, and Ben Meyer quickly went to work to rectify the situation.

Here's what was in Qt 4.3's repository as of this past Friday:

As you can see, we're basically back to what Qt 4 had, except with quick locations added to the left. The quick locations allow adding and removing, and settings are saved. Unfortunately, no renaming though, so I'll likely end up with many directories labled "src" confusing me. Also, when using the file name box to browse, the bug from the former Qt 4.3's file save name box is here. If I enter "/usr/src", it'll switch to that path, but the name box will end up stupidly containing "src". Seems like someone forgot to do an S_ISDIR(st_mode) on stat(path) before blindly filling the box with basename(path) when enter is pressed.
I have great faith in the Trolltech guys though, these guys care, and fix things promptly. Lets hope they notice this and fix it before 4.3 is ready. One neat thing about the new version though is that you never need to refresh, as the dialog monitors the directory for changes. But don't worry, the thing is lightening quick, and doesn't seem to lag for anything. I even threw it against a directory with 20,000 files, and it displayed it instantly.

Finally, regarding the KDE 3.5 dialog, I wrote last time how it was the best thing I reviewed, my only disappointment was no renaming. However I was informed that you can rename with it. When you right click on a file, the rename option is labeled "properties". Once the properties come up, you can immediately rename, however the additional benefit here is that you can also click check boxes to change the permissions on a file too! I never thought to look in properties before, as I figured it would just give me info on the file, not actually allow me to change anything. Perhaps there should be some better naming go on over there to make it more intuitive, but it is now apparent that the KDE 3.5 dialog is definitely the superior dialog I have actually reviewed.

I really like the idea of adding a search feature though, and crumb supports usefulness is debatable. So I'll toss it up between Windows 6 and KDE 3.5 as to which is the best till I get a chance to get my hands on Vista.
However, KDE 4 will probably add a search to their file open, and I expect the clever guys at Trolltech to improve further if they receive enough feedback.

If you want developers of your favorite API/OS/Desktop Environment to improve, why not point them to this and the previous file dialog reviews. The guys at Trolltech are definitely open to feedback. Just make sure you're ready for rejection if you try talking to the GTK/GNOME guys, they don't care about anything.

Quicksort

Sorting is something most programs have to do on some occasion. Quicksort is accepted as the best in-practice general purpose sorting algorithm available today. Standard C offers a built in Quicksort under the function name qsort(), with the following prototype:

void qsort(void *base, size_t nmemb, size_t size, int(*compar)(const void *, const void *));

Now while it may be built in, each C library implements it differently. Also, it's designed to work with any kind of type/structure, however one can generally optimize it a bit more for the kind that they're dealing with. Since sorting is important, and can take a while with many items, one generally wants their sorting to be optimized as much as possible.

I've looked at several implementations for various C libraries out there, however they all have one thing in common - messy. One I looked at didn't even implement Quicksort, but a Shakersort.

I'm wondering perhaps if some of the clever people who read this blog can come up with a clean but fast implementation.

To start off, here's a mostly unoptimized simple implementation of the standard qsort():

static void swap_internal(char *a, char *b, size_t size)
{
  if (a != b)
  {
    char t;
    while (size--)
    {
      t = *a;
      *a++ = *b;
      *b++ = t;
    }
  }
}

static void qsort_internal(char *begin, char *end, size_t size, int(*compar)(const void *, const void *))
{
  if (end > begin)
  {
    char *pivot = begin;
    char *l = begin + size, *r = end;

    while (l < r)
    {
      if (compar(l, pivot) <= 0)
      {
        l += size;
      }
      else
      {
        r -= size;
        swap_internal(l, r, size);
      }
    }
    l -= size;
    swap_internal(begin, l, size);
    qsort_internal(begin, l, size, compar);
    qsort_internal(r, end, size, compar);
  }
}

void qsort(void *base, size_t nmemb, size_t size, int(*compar)(const void *, const void *))
{
  qsort_internal((char *)base, (char *)base+nmemb*size, size, compar);
}

Note, this implementation is ~40 lines and can be improved. It can be improved with a better pivot selection, removing the recursion, and throwing in an insertion sort for certain cases.

I'm challenging my readers to give me a good clean implementation, which is under 80 lines (bad indentation doesn't count, I'll be running submitted code through my formatter), compliant, and isn't a total wreck. Creating some helper functions to make it clearer is a good idea too. Almost every implementation I've seen makes use of gotos to all over the place, and has returns smack in the middle of the function.

It'd be nice to bring something so useful into the new millennium, instead of using messy or slow implementations from the 70s.

If I get some nice implementations in, I'll do another piece on how to optimize Quicksort for sorting certain types/structures.

Methods for safe string handling

Every now and then you hear about how a buffer overflow was discovered in some program. Immediately, everyone jumps on the story with their own take on the matter. Fans of a different programming language will say: "of course this wouldn't have happened if you'd have used my programming language". Secure library advocates will say: "you should have used that library instead". While experts of that language will say: "The guy was an idiot, he coded that all wrong".

I'd like to look at basic "C string" handling in C. We're talking about functions like strlen(), strcpy(), strcat(), and strcmp(). These functions report length, copy, concatenate, and compare C strings respectively. A C string is an array of characters, which is terminated with a null character to signify the end. strlen() would find the length by seeing how far from the beginning the null was. strcat() would find the null in the first C string, and then to that location copy characters from the second string, till it finds the null. So on and so forth with all the C string functions.

Now these functions are seen by some as inherently broken, since they can read/write data right off the end of the buffer, and the terminating nulls can sometimes vanish if one isn't careful. You see these kinds of issues with C++ programs too. Usually when a Java programmer hears about this, they tell you to use Java, since Java has a built in string class which can't get screwed up by any of these simple operations. Learned C++ programmers know that C++ also has a built in string class, just as good, if not better than Java's string class. Knowledgeable C++ programmers will use C++'s strings to avoid all these issues, and have really nice features, such as sane string comparison using ==.

Switching from C++ to Java, because people know that Java has a string class is kind of ridiculous. Yet I keep hearing from all kinds of people how programmers should switch to Java because of it. One using C++ should generally use the C++ class, unless they have a good reason otherwise. It's a shame most C++ programmers use C strings instead of C++ ones, probably due to them not knowing about it. However this doesn't help when programming in pure C.

For pure C, you can turn to one of the libraries for handling strings, such as SafeStr. But most people won't choose to go this route. Due to this, care has to be taken.

Now I won't kid you, if there's a buffer overflow in a C program, it is the programmer's fault. If s/he was more careful, it wouldn't have happened. However some areas of code are large, have many code paths, and are downright confusing. In those cases, it's easy to screw up. To help prevent screwing up, there exist some more C functions to handling strings properly, and some C libraries also provide extra non standard functions.

In answer to just overflowing a buffer when copying or concatenating, "n" versions are provided. These are strncpy(), strncat(). They take a third parameter to tell it how much they're dealing with. strncpy()'s n refers to how big the buffer is, and it'll only copy up to that amount of characters. strncat()'s n is up to how many characters can be stuck onto the end of the first string. In the case of strncpy(), if null isn't found in the copy process, the result is not null terminated. Leaving us with the other problem. strncat() on the other hand will always null terminate, because it attaches n+1 bytes whenever the second string's end isn't reached. Meaning that you have to tell strncat() length_of(remaining bytes in buffer)-1. This leads to confusion, because of different n meanings, and because strncpy() introduces another problem.

There's also strncmp(), for specifying the maximum amount of characters to compare, which you can use if one of the strings isn't null terminated. Surprisingly however, there is no strnlen(), to check how many characters a string is, without running off the end. Considering that strncpy() doesn't always null terminate, sounds like a useful feature to have.

Taking this into account, in some C libraries, you'll find strnlen() which returns the length, or instead will return the value of n, in the case where no null was found. Those needing it, it's an easy function to implement yourself:

size_t strnlen(const char *s, size_t n)
{
  const char *p = (const char *)memchr(s, 0, n);
  return(p ? p-s : n);
}

Although it would be intelligent to follow up every call to "strncpy(s, n)" with a "s[n-1] = 0" to terminate it yourself. But this hardly helps the confusion. Also take into account that str[n][cpy/cat]() return the destination for their return value, so you'll sometimes see code like:

if (!strcmp(strncpy(buf, entered_text, sizeof(buf)), param))
{
  do_something();
}

However, this code is broken as buf may not be null terminated. A correct version would perhaps be:

if (!strncmp(strncpy(buf, entered_text, sizeof(buf)), param, min(sizeof(param), sizeof(buf))))
{
  do_something();
}

Which is ugly at best.

To solve and work around these issues, OpenBSD has invented the strlcpy() and strlcat() functions, which have been implemented in all BSD derivatives (including Solaris and Mac OS X). Manpage here.

Although I found the standard descriptions confusing at best. Here's my take on it after some study:

size_t strlcpy(char *dest, const char *src, size_t size);

Description:
    The strlcpy() function copies the C string pointed to by src (including the null) to
the array pointed to by dest. However, not more than size bytes of src are copied. Meaning
at most size-1 characters will be copied. The copy will always be null terminated, unless
size was 0, in which case nothing is done to dest.

Return Value:
    Return is always the amount of characters needed to hold the copy. Meaning strlen(src).
If the return value is <size, everything was copied.

With strlcpy(), you can run it once with a size of 0 if you want to find out how much you need to allocate. Although pointless, you'd be better off with strlen(). Now this won't help much if src isn't null terminated, but it should avoid issues you have with misusing the return value like in the case offered above, or when there wasn't enough room. If you always pass strlcpy() the sizeof() the buffer, or the value passed to malloc() as the case may be, you should be safe.

If you read the manpage, you also see a usefulness to the return value. A problem with constantly using strcat() is that you have to keep iterating through the former strings, leading to a speed loss. With strlcpy(), you can do the following for concatenation:

if ((n1 = strlcpy(a, b, sizeof(a))) < sizeof(a))
{
  if ((n2 = strlcpy(a+n1, c, sizeof(a)-n1)) < sizeof(a)-n1)
  {
    if ((n3 = strlcpy(a+n2, d, sizeof(a)-n2)) < sizeof(a)-n2)
    {
      etc...
    }
  }
}

A nice trick for mass concatination, although as the manpage points out, ridiculous, and negates strlcat().

Moving onwards, the manpage listed above for strlcpy() is also for strlcat(), yet as above, I found it a bit confusing too. Here's my take:

size_t strlcat(char *dest, const char *src, size_t size);

Description:
    The strlcat() function appends the src string to the dest string overwriting the ‘\0’
character at the end of dest, and then adds a terminating ‘\0’ character. However, not more
than size-strlen(dest) bytes of src are copied. Meaning a maximum of size-1 characters will
fill dest in the end. The copy will always be null terminated, unless size was less than the
length of dest, or dest is not null terminated, in which case nothing is done to dest.

Return Value:
    Return is the amount of characters needed to hold the copy when dest initially is null
terminated and its length is less than size. Otherwise the return is size+strlen(src). If
the return value is <size, everything was copied.

What's nice about strlcat() is that for the size param, you can pass it the sizeof() or the malloc() value like you do for strlcpy(). But beware the return value, the OpenBSD code is rightly commented as follows: "Returns strlen(src) + min(siz, strlen(initial dst))". Take a moment to comprehend that.

If you're not using a BSD and you want these functions, code is here and here. Be wary of some of the other implementations you find online. I looked at some of them, and they acted differently in some other corner cases. One I looked at even crashed in one of the corner cases.

However looking at that code there, it looks a bit messy. Reviewing our previous multiple concatenation case, which is also spoken about in the manpage, one sees these as a bit weak. If one wants nice multi concat without too much fuss, they'd normally use snprintf() (C99) with a bunch of "%s%s%s" as the format. I myself though prefer a more elegant solution to all of this.

I therefor have created the following logical extension of OpenBSD's l functions, I give you strlmrg():

size_t strlmrg(char *dest, size_t size, ...)
{
  char *s, *end = dest + (size-1);
  size_t needed = 0;

  va_list ap;
  va_start(ap, size);

  while ((s = va_arg(ap, char *)))
  {
    if (s == dest)
    {
      size_t n = strnlen(s, (end+1)-s);
      needed += n;
      dest += n;
    }
    else
    {
      needed += strlen(s);
      if (dest && (dest < end))
      {
        while (*s && (dest < end))
        {
          *dest++ = *s++;
        }
        *dest = 0;
      }
    }
  }

  va_end(ap);
  return(needed);
}

Pass strlmrg() the destination buffer, it's size (from sizeof() or the param to malloc()), and all the strings you want concatenated, followed by a null pointer.
Example 1:

printf("%zu; %s\n", strlmrg(line, sizeof(line), "I ", "Went ", "To ", "The ", "Park.", NULL), line);

It would print: "19; I Went To The Park."
Example 2:

n = strlmrg(buffer, sizeof(buffer), a, b, c, d, e, f, (void *)0);

Which would concatenate a to f inside buffer (given that it could fit), and return the amount of characters copied. Note, it returns how many characters would be copied, so you can use it to determine the size. See this example:

size_t n = strlmrg(0, 0, a, b, c, (void *)0);
char *p = malloc(n+1); //+1 for the null
strlmrg(p, n+1, a, b, c,(void *)0); //Again, +1 for the null

When strlmrg() returns less than size, everything was merged in. The result is always null terminated except when dest is null, size is 0, or it encounters one of the source pointers to match the location it is currently trying to copy to.
You should avoid passing one of the source pointers to be a location from the destination buffer. If you happened to pass in such an overlapping source pointer, and it's not null terminated prior to it reaching size, you will get size as the return value instead of the full size. Also don't try to pass it any non null terminated source pointer, or forget to pass the last null pointer.

Once we have strlmrg() implemented, it also paves the way for a simple and straightforward implementation for strlcpy() and strlcat().

size_t strlcpy(char *dest, const char *src, size_t size)
{
  return(strlmrg(dest, size, src, (void *)0));
}

size_t strlcat(char *dest, const char *src, size_t size)
{
  return(strlmrg(dest, size, dest, src, (void *)0));
}

And unlike the official ones, these won't crash if dest or src is null. I tested these wrappers, and they seemed to match results with the official ones in every regular and edge case I tried.

I also tested strlmrg() in a variety of cases, and it seems to be very good and secure. If you find a bug, or have an improvement to offer, feel free to post about it.

Thoughts?

Are open source libraries written properly?

Every now and then you hear people discussing open source applications and libraries, and how a bug was found. The multitude of bugs being found makes one wonder are open source libraries being written properly? Are they secure?

It is incumbent upon one to realize that having the code open makes it easier for someone to find bugs. There can be many bugs in closed source libraries that we simply don't know about. When the source is open, bugs can be found and immediately fixed by anyone. It leaves the question which one is better?

I'd like to break down bugs being found to two different types. Type A bug is where the library is very complex, and one unique circumstance with a certain corner case of data will expose a flaw. Type B bug is where the author isn't very good at writing libraries, or not familiar with the underlying system calls he's working with, and writes the code all wrong. We'll look at both kinds of Type B bugs shortly.

Type A bugs can exist in open or closed source libraries, but since they're so hard to pinpoint, they rarely are found. In this instance, open source can be an advantage, as one can go over the code with a fine tooth comb and look for that rare case, or the user of an app may be getting some inexplicable behavior, and looking at the source with the given data can track it down. Open source helps the application developer with getting this kind of problem fixed. In a closed source library, a user can be experiencing an error, and the developer of the application has no way to track it down, leaving a bewildered developer and an annoyed user.

Type B bugs can exist in both as well. However a closed source library, which costs money, normally doesn't last long if it has stupid bugs in it. In this instance, it generally is a bit rarer to find this kind of bug in a successful established closed source library. Regarding open source however, if the library isn't popular enough that various groups are trying to attack it, these bugs can go unnoticed, and therefor unfixed for a long time. Perhaps the mentality when seeing such an obvious bug causes one to think: "it's open source, this probably is correct as many other people have reviwed it already", which is a very bad conclusion.

While looking at some shared library code that you can find in GNU libraries used by the latest versions of coreutils, tar, and others, I found some Type B bugs.

I was using Google Code Search the other day to see how various groups implemented certain functions. While looking up three different functions, I found two of these to have bugs in them.

The first was in "lib/save-cwd.c", I found this:

  cwd->desc = open (".", O_RDONLY);
  if (cwd->desc < 0)
    {
      cwd->desc = open (".", O_WRONLY);
      if (cwd->desc < 0)

This bit of code tries to first open the current directory in read only mode, and assign the handle to "cwd->desc", if that fails, it retries in write only mode, and if that fails, it does something else.
Now I have to wonder if the people who wrote and reviewed this file have a clue what they're doing. It isn't possible to open a directory in any mode but read only. Manual for open() clearly lists the following error: "EISDIR - pathname refers to a directory and the access requested involved writing (that is, O_WRONLY or O_RDWR is set).", meaning if write mode of any kind is specified when trying to open a directory, it will fail.
There is no reason in the world any developer should ever try to open a path they know is a directory with open() and any kind of write mode. What's worse is that they try to use it in an error handling condition, and on top of that have some if which acts like it may succeed.

The other bit of code I found was in "lib/atexit.c", here it is:

int
atexit (void (*f) (void))
{
  /* If the system doesn't provide a definition for atexit, use on_exit
     if the system provides that.  */
  on_exit (f, 0);
  return 0;
}

For some background, here is the details for atexit():

The atexit() function registers the given function to be called at normal process termination, either via exit(3) or via return from the program’s main(). Functions so registered are called in the reverse order of their registration; no arguments are passed. The atexit() function returns the value 0 if successful; otherwise it returns a non-zero value.

And on_exit():

The on_exit() function registers the given function to be called at normal process termination, whether via exit(3) or via return from the program’s main(). The function is passed the argument to exit(3) and the arg argument from on_exit(). The on_exit() function returns the value 0 if successful; otherwise it returns a non-zero value.

The functions look pretty much similar, the only difference being that on_exit() seems to have an extra parameter. Now ignoring that extra parameter, shouldn't one be a pure wrapper to the other? Why is the code for atexit() calling on_exit(), then always returning success even when on_exit() failed? A more appropriate implementation would be:

int atexit (void (*f) (void))
{
  return on_exit (f, 0);
}

Is that really so hard to write properly?

Seeing rampant stupidity and bad code in these GNU libraries, you know something over there isn't doing too good. I only found these issues in reviewing 3 functions in less than 5 minutes. I'd suggest avoiding GNU libraries like the plague.

If you're going to be using an open source library for a simple operation, I highly recommend reviewing the code yourself to make sure the original developers had a clue what they were doing.