Bad Shell Scripts

I’m always amazed at the number of bad shell scripts I keep coming
across. Take this snippet for example:

TEMP1=`mktemp /tmp/tempenv.XXXXXX` || exit 1
perl $CONF_DIR/project.pl $@ > $TEMP1
if [ $? != 0 ]; then
  return
fi

. $TEMP1
rm -f $TEMP1

There are several things wrong with this. First it uses a temporay file.
Secondly it uses more processes than are required and thirdly it doesn’t
clean up after itself properly. If perl failed, the temp file would
still be created, but not deleted. The last problem can be solved with
some suitable uses of trap:

TEMP1=`mktemp /tmp/tempenv.XXXXXX` || exit 1
trap "rm -f $TEMP" INT TERM EXIT
perl $CONF_DIR/project.pl $@ > $TEMP1
if [ $? != 0 ]; then
  return
fi

. $TEMP1
rm -f $TEMP1
trap - INT TERM EXIT

Of course this can all be replaced with a single line:

eval $(perl $CONF_DIR/project.pl $@)

Web logs considered disturbing

I installed awstats today for the first time on my personal domain and
reading though the search terms is always amusing, but I got concerned
by people finding my site searching for “bonnie langford naked” and
“naked bonnie langford”. Turns out I was talking about Bonnie Langford
in a post about Charlie and the Chocolate Factory and the naked thing
was to do with body scanners on the London Underground.

But what really worried me was the search for “scott james remnant
naked”.

Mozilla causing XSS in Livejournal

As mentioned in my last entry, Livejournal have changed their url scheme to get around a problem in mozilla. The trouble is down to the -moz-binding CSS property. I’ll repost a copy of the short lived posting to the lj_dev community earlier.

Discussing the URL Change

As we recently announced in

news, we have changed the canonical URL of most journal, community, and syndicated content. While we did offer subdomains to Paid users in the past, this is now the canonical URL that we will link and redirect to for all journals. However, all communities are located at http://community.livejournal.com/username and syndicated accounts at http://syndicated.livejournal.com/username. Due to the way certain proxy servers are configured, and the fact that the HTTP RFC prohibits it, the canonical URL for journals with a username starting with an underscore will be http://users.livejournal.com/username. We are however offering a free rename if you fall into this group.

So now the technical side of all of this and why it was a required change; bear with me as its 3am my time and I’m sitting in John F. Kennedy Airport after a five hour flight. Late last week we become aware that it was possible to use the “-moz-binding” CSS attribute within Mozilla and Mozilla Firefox to execute arbitrary offsite JavaScript. As this attribute is designed to allow attaching an XBL transform and JavaScript to any node within the DOM, it is quite easy to use in a malicious fashion. We immediately altered our cleaner to strip this attribute from entries and comments, though also realized that wasn’t even half the battle.

As we allow custom CSS in many of our styles, as well as the ability to link to an external stylesheet in a variety of fashions, it was quite possible to take advantage of this exploit and hijack the session cookie of any user who views your journal. As we, along with many other sites, used one cookie to authenticate a user, this cookie was quite powerful if stolen. If the user had not chosen to bind their cookie to their IP address, a malicious user could steal it, login as that user, deface the account and SPAM with it, as well as modify that user’s style to include the exploit thus causing this problem to spread much like a virus.

Borrowing the idea from another development team within Six Apart, we decided we needed to break our cookies into three categories. One cookie would be our application management cookie, this cookie would only be accessible on www.livejournal.com where we will not display untrusted content. A second cookie will be accessible on all subdomains of livejournal.com, though it only will say if you are logged in or not; it is solely for optimization. We then will issue one cookie for each journal you visit. This cookie will be only accessible on username.livejournal.com or community.livejournal.com/username as it is limited to a single journal. This cookie will only grant you the permission to read protected entries and post in the particular journal. This means that if the journal owner steals your cookie, they will be able to do nothing more than view their journal and comment upon it as if they are you. In the end you will have n+2 cookies, with n being the number of journals you visit.

Due to the fact that we cannot clean every external CSS stylesheet linked to every time we generate a journal page, this change is required. While it does not fully protect us from some new cross site scripting vulnerability that can be exploited via entries or comments, they are much easier to block, patch, and recover from quickly. With Mozilla deciding to allow the execution of arbitrary JavaScript via CSS, there is no other viable solution than the one we have undertaken. We developed a plan to phase all of this in over the next week, URL change first and then followed by the cookie change, though this morning we were made aware that this was being actively exploited. As such we took our week time line and shortened it to about twelve hours. While URLs have been changed at this time, our cookie handling change has not yet happened. This should however be expected to take place within the next day or two as well as various other cleanups and fixing bugs we’ve already encountered.

Please feel free to let us know if you have any questions and open a support request if you find a bug or encounter a problem. Sorry all of this came with just about zero warning, but in the end we could not wait longer to fix this problem.

Thanks to Daniel Silverstone for providing me with a cache copy of the text.

Unintended Consequences of Technology

I’ve been meaning to post this for a couple of days. I discovered a
rather unintended and quite unexpected consequence of using a piece of
software. At work I use Workrave to enforce
breaks from the computer. For those that have never used it, workrave is
a little gnome applet that monitors your keyboard and mouse usage and
after about an hour of work locks your screen and gives you a 10 minute
break with a few exercises to reduce RSI. It also gives you microbreaks
of 20 seconds every 3 minutes of work, so you can look away from the
screen, stretch, whatever.. I should point out that it isn’t 20 seconds
every 3 wallclock minutes, but 3 minutes of using the mouse/keyboard and
isn’t quite as intrusive as it sounds.

I get to rest my eyes, prevent RSI annd take breaks. This sounds like
exactly the thing Workrave was written for. So what is unintended? It
turns out that the 20 seconds micro-break is just the right amount of
time to have a drink of water or get up and fill my glass if it’s empty.
Before I would forget to fill my glass up, but now I have time where I
can’t do anything on the computer. In the 30 minutes since I got in to
the office and started writing this entry I’ve had two micro-breaks and
drunk a pint of water. Workrave has managed to increase my water intake
during the day and as a result the number of toilet breaks. 🙂

I notice LiveJournal have
enforced http://<username>.livejournal.com/ url scheme now to
prevent cross-site scripting attacks where people were stealling session
cookies and gaining access to accounts. Not sure what the exact problem
was, but I know of several attacks over the last week by one group of
scriptkiddies.

Update: Appears the problem was in firefox. Explaination
here.

Update: Nope, they pulled it. Basically mozilla allows you to execute
javascript via CSS stylesheets. I’ll update with any further URLs as
they become available.

I had an epiphany in the shower

About a year ago I had a problem with udev crashing during startup on
my powerpc box. Somehow I managed to muddle on with this problem,
probably by not rebooting the box. 🙂 Last summer I had to reoot it
again so I did a bit more research and discovered that udev was trying
to looking up the nvram group, not finding it in /etc/group and then
trying ldap, which, of course, failed because we have no networking yet.

Adding the group fixed the bug and filed a bug
against udev saying that udev should add any groups it used. Carrying
out further debugging revealed that the crash was during nss_wins. The
general order of events were:

  1. udev looks up a user or group.
  2. Group doesn’t exist in compat.
  3. Lookup in ldap.
  4. Ldap attempts to resolve the name of the ldap server or client.
    (server is 127.0.0.1 so confused about this point.)
  5. Network and/or dns server isn’t up so dns fails fails.
  6. Attempts to look up host in wins.
  7. udevstart crashes.

I didn’t have time to debug this any further and proceeded to forget
the problem, but last night my fileserver started having the same
problem. Removing ldap from passwd, group and shadow resolved the udev
problem, but then I didn’t have any users. Late last night I booted
without ldap and then changed nsswitch.conf to add ldap, and went to
bed.

This morning I had an epiphany in the shower. Not only did I remember
what the bug was, but also a sensible workaround. The problem wasn’t
with the passwd et al lines, but the hosts line. I did have

hosts: files dns mdns wins

The solution is to return if dns isn’t available and changed the line
to:

hosts: files dns [UNAVAIL=return] mdns wins

Now all I need to do is to debug nss_wins and get to the bottom of
the crash. It might be worth filing a bug against nss_ldap for trying to
do a lookup against an ip address.

OOP Strikes Back

Steinar,
you missed my point. I explicitly said not to get bogged down in my
example. The point is that changing the internal details of your class
shouldn’t result in your users being forced to change their code. As I
said, exposing your implementation tightly couples your code and your
users. As Matthew
pointed out, you shouldn’t have sets and gets for every item in your
class. You should have a clearly thought out api that isn’t tied to the
internal detail. Exposing internal, either through making data public or
blindly creating gets and sets for private data members is just bad
OO design.

An addition to my example that makes things little cleared would be a
subclass that validated the words you could use. With a public variable
I couldn’t enforce that, but I could override my setter to add
validation.

Return of Accessors vs. public member variables

Steinar,
Imagine a Word class: 

class WordA {
   public:
      char * word;
};
class WordB {
   private:
      char * word;
   public:
      void setWord(char * w) { word = w; }
      char * getWord() { return word; }
};

WordA worda;
worda.word = "WordA";

WordB wordb;
wordb.setWord("WordB");

printf("%s %s", worda.word, wordb.getWord());

Fine, this works well. Now imagine that you want to change word from a
char * to a std::string to stop you dealing with
pointers. For the WordBclass you only need to change the
getWord function. You don’t need to change any of your class’s
users. For the WordA class, you have a problem because you can’t
automatically convert from a std::string to a char *,
so all your users have to change from object.word to
object.word.c_str(). 

class WordA {
   public:
      std::string word;
};
class WordB {
   private:
      std::string word;
   public:
      void setWord(char * w) { word = w; }
      char * getWord() { return word.c_str(); }
};

WordA worda;
worda.word = "WordA";

WordB wordb;
wordb.setWord("WordB");

printf("%s %s", worda.word.c_str(), wordb.getWord());

This is the punishment you get for exposing the internal details of
your class to your users. Please ignore any specific mistakes in my
examples; the principles work the same with different types.

Accessors vs. public member variables

Steinar, you use accessor rather than public data members because you
may need to change the behaviour of the class to do something when you
set a member variable. If you have all your data public, you can’t do
this. If you force people to go via a function, you can make changes to
the class without affecting its users. You have a similar issue with
inherited classes.