bug-cvs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

cvs 1.11.2: security issues


From: Nick Cleaton
Subject: cvs 1.11.2: security issues
Date: Thu, 23 May 2002 22:02:35 +0100
User-agent: Mutt/1.2.5.1i

Hi,

While looking for security problems in cvs-1.11.2, I found
a few issues:


1) Heap underflow in pserver.  From cvs-1.11.2/src/subr.c:

  /* Remove trailing newlines from STRING, destructively. */
  void
  strip_trailing_newlines (str)
       char *str;
  {   
      int len;
      len = strlen (str) - 1;
  
      while (str[len] == '\n')
          str[len--] = '\0';
  }

When this is passed a string of zero length (which an attacker
can arrange, by putting a null byte at the start of the repository
string in a pserver connection) it accesses the character
before the start of str, and changes it to a null if it's a
newline.

I don't think this is exploitable on FreeBSD where I tried it,
but I can think of ways to implement malloc that would make it
exploitable for arbitrary command execution.


2) No check for ':' in usernames.  From cvs-1.11.2/src/server.c:

    /* Look for a relevant line -- one with this user's name. */
    namelen = strlen (username);
    while (getline (&linebuf, &linebuf_len, fp) >= 0)
    {
        if ((strncmp (linebuf, username, namelen) == 0)
            && (linebuf[namelen] == ':'))
        {
            found_it = 1;
            break;
        }
    }

So a username like "foo:*" could match more of the line in the
password file than it should.  I can't think of a scenario
where that causes a problem, but it's still worth fixing IMO.


3) The assert() macro is used to prevent buffer overflows in a
few places.  This is a bit fragile since on FreeBSD the assert()
macro does nothing if the NDEBUG preprocessor symbol is set.

An example from cvs-1.11.2/src/server.c:

    /* The client will send us a two byte length followed by that many
       bytes.  */
    if (fread (buf, 1, 2, stdin) != 2)
        error (1, errno, "read of length failed");

    nbytes = ((buf[0] & 0xff) << 8) | (buf[1] & 0xff);
    assert (nbytes <= sizeof buf);

    if (fread (buf, 1, nbytes, stdin) != nbytes)
        error (1, errno, "read of data failed");


--
Nick Cleaton
nick@cleaton.net



reply via email to

[Prev in Thread] Current Thread [Next in Thread]