On Fri, Sep 05, 2008 at 10:03:27AM +0200, Daniel Stenberg wrote:
> I'm going to do a talk[*] on security in "popular" open source software in
> a while, with stories and experiences from our little project.
>
> So I'm curious on how you app authors feel about security and the curl
> project, in general and specific cases. I feel that I have a view on this
> from an author's perspective and I have opinions about what we (can) do to
> make curl and libcurl remain safe and secure. But how do you users of
> (primarily) libcurl view us; the code, the project, the product from a
> security perspective?

I have some opinions on this from the perspective of having hacked the
libcurl code somewhat. Many of the points I'm about to make aren't
specific to security, but are general problems that can result in
introduced bugs, many of which could have security implications.
curl deals with untrusted input from random remote servers so there are
many classes of bugs that can be exploited remotely (even if such bugs
only result in a crash, that's still a denial of service problem that
falls under the security heading).

The main issue I see with curl is the age of the code base. It's over ten
years old now and has been in continual development during that time.
Features have been tacked on one at a time, often through patches
created by people who aren't very familiar with the code or its design.
Over time, this results in effects like:

- Functions end up being much too long (I recall seeing a few at close to
1000 lines) as they're patched and expanded--I've found several
bugs by simply refactoring long functions into several smaller ones.
One problem had to do with local variables being declared hundreds of
lines before use; this poor scoping made it hard to see problems with
initialization and can allow dangling pointers to persist in too large
a scope.

- Duplicated code. A bug found and fixed in one place isn't always fixed
in another, and the code ends up being bloated more than necessary. The HTTP
tunnel proxy code for example ends up with some duplication with the normal
HTTP proxy code. The easy interface also duplicates some code in the
multi interface; the easy perform loop could be rewritten to use the multi
code internally to avoid this duplication.

- Poor locality of reference and poor cohesion. Parts of a sets of related
pointers and sizes end up being manipulated by wildly separated sections
of code. Pointers and other variables end up not always reinitialized
properly. This can be especially problematic since one handle can result
in several requests being made, and can each can be restarted from one
of several places in the code.

- Boundary conditions being missed, which is complicated by related logic
being spread out over several functions.

- There is a huge amount of state that needs to be kept around. This is
increased due to the multi interface's requirement to exit processing and
return control to the application frequently, instead of just blocking
when new data is needed and keeping the flow of control more obvious.

There are plenty of things done right, of course. The test suite goes a long
way towards finding many classes of bugs that can contribute to denial of
service issues. Finding some way to incorporate fuzzing into the test suite
would help even more. The test suite also provides some reasonable level of
assurance that refactoring the code to help address certain issues won't
cause regressions. The refactoring that took place in the FTP code, and
later the SFTP code, are good examples of this.

The decision to split the internal data into related structs also helps
to improve cohesion. But more could be done to ensure that data is
correctly initialized and destroyed in all cases.

The kind of security issues I've seen lately in curl are far from obvious
ones. It takes plenty of patience to find them. It's a good sign that
not many security problems have been reported in curl, but I fear that
there are some latent issues hiding in the code, waiting for someone with
patience to ferret them out. I also believe that spending more time
refactoring the code to smooth out some of the hairier parts is worth the
time in improved maintainability as well as improved security.

>>> Dan

-- 
http://www.MoveAnnouncer.com              The web change of address service
          Let webmasters know that your web site has moved