Attachment 32729 Details for Bug 74331

Additional comments from Derek Price

derek (text/plain), 4.93 KB, created by Ludwig Nussel on 2005-03-24 10:19:09 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Ludwig Nussel

Created: 2005-03-24 10:19:09 UTC

Size: 4.93 KB

patch

obsolete

>-----BEGIN PGP SIGNED MESSAGE-----
>Hash: SHA1
>
>Mark J Cox wrote:
>
>|> The attached patch wants at least one CVE #.
>|
>|
>| The document lists a lot of memory leaks which could be a minor
>| security consequence say if a remote authorised CVS user could DoS
>| the server.
>
>
>A remote user could definitely have exploited at least several of
>these memory leaks to the point of DoS.  I wasn't considering this as
>very dangerous and just committed the fixes to the public CVS tree as
>I was working on them.  They probably must be considered as released,
>even if probably without a wide distribution as yet.  Others may very
>well want to pull those patches for their next security release.  My
>apologies for not holding them for coordinated release.
>
>I also committed several fixes for NULL pointer references and
>assertions that other NULL pointer references would not occur since I
>perceived most of these as only exploitable for denial of service as
>well.  Should I have withheld these as well?  Most of these did
>require corrupt RCS archive files for exploit, which would usually
>require a CVS server that had already been compromised.
>
>| For each of the issues you're patching we'd need to go through them
>|  and look at what possible attack paths exist to see if they really
>|  have a security context then assign CVE names based on that.
>
>
>Notes below.  On further consideration, perhaps only the buffer
>overflow is worisome.  Comments?
>
>| I know folks on this list are busy with a number of significant
>| issues, so setting a release date a couple of weeks out would be
>| really appreciated and give us all time to go through this, do the
>| backport, allocate names, and so on.
>
>
>Is Monday, April 11th alright?  I'd be ok with Monday, April 18th, as
>well.
>
>| +    * login.c (password_entry_parseline): Avoid using uninitialized +
>| variable.
>
>
>This one probably isn't exploitable.  It would be a client exploit in
>the first place and requires access to the client's ~/.cvspass file to
>do it.  I probably shouldn't have included this one in the security
>patch.  I just drew an arbitrary line with uninitialized variables and
>buffer overflows on one side and the other stuff on the other.
>
>If you all agree I'll remove this from this patch and commit the
>change to the CVS sources.
>
>| +    * rcs.c (RCS_deltas): Avoid buffer overflow.
>
>
>This one *is* probably exploitable, as I understand things, but I am
>no security guru.  The text should not be completely configurable from
>the client side, though it may be somewhat if the user has commit
>access.  Text is being printed into an 80 character long buffer with
>the following command:
>
>~    sprintf (buf, "%-12s (%-8.8s ",
>~             prvers->version,
>~             prvers->author);
>
>Setting AUTHOR here probably requires admin access to the CVS server
>anyhow.  This might be available via another (known) exploitable hole
>in the pserver protocol if the user can write to the CVSROOT/passwd
>file, but if they could do this then it is likely that they could
>already execute arbitrary data on the CVS server as arbitrary non-root
>users and would have no need of this exploit.
>
>Setting VERSION would be easier.  The version only has to exist in the
>RCS file, so if the "long" revision does not exist already, then
>anybody with commit access should be able to create an arbitrary
>revision of the form "N.N(.N.N)*", where Ns may be any integer > 0,
>with some sequence of commit commands.  Whether a string of this form
>written over memory could be made useful to an attacker, I do not know.
>
>| +    (RCS_checkout): Avoid using uninitialized loglen.
>
>
>This should not be easy to exploit remotely as it requires a corrupt
>RCS archive file on the server.  This should not be possible unless
>the server has already been compromised.
>
>| +    * patch.c (patch_fileproc): Free original pointer, not one that
>| may +    have been incremented.
>
>
>I made this fix without examining the code path.  Simply based on the
>fact that this code can strip portions of the path based on the file
>name and the repository name caused me to assume that there was a path
>that could cause the path to be stripped.  Once the path is stripped,
>the free of an unallocated pointer will always occur.
>
>On further examination, though, looks like dead code.  RCS is copied
>from FINFO->FILE, which should never have any directory elements, but
>should only be a file name.  I backtracked a little for a way to sneak
>a directory element into FINFO->FILE and couldn't find one.
>
>It's still possible that there is an attack vector, but on deeper
>examination, I can't find it, and think it may actually be impossible.
>
>Cheers,
>
>Derek
>-----BEGIN PGP SIGNATURE-----
>Version: GnuPG v1.4.0 (Cygwin)
>Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org
>
>iD8DBQFCQZWgLD1OTBfyMaQRAoEsAJ9YFc0p8wsFRihfpPdfmxWss2pYhgCg4K3S
>QKQN8afeZ3hf9SBf54XNSxY=
>=PEn+
>-----END PGP SIGNATURE-----
>
>
>_______________________________________________
>Vendor Security mailing list
>Vendor Security@lst.de
>https://www.lst.de/cgi-bin/mailman/listinfo/vendor-sec

Actions: View

Attachments on bug 74331: 32647 | 32648 | 32729 | 33211 | 33212