Archives

KDE-CVS-Digest

Weekly View of KDE CVS commits

December 13, 2002



This Week:

The security audit, a new Release Dude for 3.2, bugfixes and new features in kmail, kopete, konqueror, konstruct and klaptopdaemon.

New stuff

I guarantee that I will miss some important code additions. There are hundreds of commits each week, and I pick out those that catch my eye. I haven't mentioned the work of all the translators, nor many of the secondary applications in kdenonbeta that have one active developer, and are making real progress. If there is something important that I have missed, please let me know.

I also want to thank the developers whom I have contacted individually for their prompt responses.


3.1 Release Delayed

Dirk Mueller announced:

The KDE 3.1 release has to be delayed further. Here is why.

On November 26th, we've been notified by FozZy from the "Hackademy Audit Project" about security problems in KDE. They can, after user interaction, cause unwanted execution of commands with the privileges of the user who runs KDE. We fixed those on the same day and updated the "hopefully final" KDE 3.1 tarballs. Unfortunately, it was becoming clear after a quick search in the KDE CVS that the problematic code is repeated in many places and in many variations.

Yesterday, on the targetted announcement date of KDE 3.1, Waldo and I realized that while we only had audited maybe 30% of the code yet, we have found enough occasions for them to be a big showstopper.

A short query on the packagers mailinglist showed that for the majority there is no big pressure on having a KDE 3.1 to be released according to the schedule. I'm considering a 3.1 with known security bugs a no-go anyway, even though we first thought that those are minor that the fix can wait for 3.1.1, I no longer think that this is the case.

Waldo, George, Lubos and I think that we can finish the audit by middle/end of next week. This however brings us in a bad position: its unlikely that we get many binary packages so short before christmas holidays, which means that KDE 3.1 would go out, if released this year, probably with few or none binary packages at the announcement date.

So, to sum up, we have two options:

a) Try to finish ASAP and try to get it out before christmas. December 12 could be a good tagging date.

b) Take the time and schedule for a release next year. Something around January 8, 2003 sounds like a good candidate (tagging date, announcement rougly a week later)

I neither like any of them, but I prefer to go with b), as it also allows for other bugs which have been reported to be fixed. For an impression just have a look at the lately steadily rising open bug count on http://bugs.kde.org/.

In any way I'll tar up and release the current 3_1_BRANCH as 3.1RC5 in a few hours. Many fixes for the above mentioned security problems are in there, but there are still big chunks of code and patches pending for review. There will be no binary packages as those which were made during the last week refer to be "KDE 3.1 final" and are anyway not up to date.

As soon as the code review is finished we will have to release updates for KDE 3.0.x (and at least patches for KDE 2.x) anyway.

Comments, opinions, suggestions, flames welcome.

Kde 3.1 RC5 is available for testing. Remember to report the bugs you find at bugs.kde.org.

What has been accomplished since then? There were literally hundreds of commits over the past week. They started with checking system() calls, then the audit proceeded onto other possibly insecure calls. /kdesecurity/review/docs/CALLS gives an incomplete list of calls to look for:

system()
popen()
mktemp()
tmpnam()
sprintf()
sscanf()
gets()
strcpy()
strcat()
memcpy()

format string vulnerabilities:
*printf()
*scanf()
syslog()
strptime()

Shell execution:

KRun::run
KShellProcess
KProcess::setUseShell

A very noticeable header has been showing up on kde-cvs this week. This is what it looks like: kdebase/ksysguard/ksysguardd [POSSIBLY UNSAFE] The cvs server is set up to run a script when a commit is made. It among other things sends off a mail message to the kde-cvs list. The perl script contains this subroutine:

sub check_for_unsafety($)
{
  my @stuff = @_;
  my $found = ""; 
  foreach my $current(@stuff) {
    next if ($current =~ /^\s*\/\//);
    $current =~ s/\"[^\"]*\"//g;
    $current =~ s/\/\*.*\*\///g;
    $found .= "$&," if ($current =~ /\b(KRun::runCommand|KShellProcess|setUseShell)\b/ and $found !~ /$&/);
    $found .= "$&," if ($current =~ /\b(system|popen|mktemp|tmpnam|gets|syslog|strptime)\b/ and $found !~ /$&/);
    $found .= "$&," if ($current =~ /(printf|scanf)\b/ and $found !~ /$&/);
  }
  chop($found);
  return $found;
}

If the commit contains the calls listed in the code fragment, the script then appends [POSSIBLY UNSAFE] onto the end of the cvs directory name. I asked Dirk Mueller about this change:

> Does the cvs client show this as some kind of error as the developer does his commit?

Nope. it only appears on kde-cvs. it was a floating idea somewhen to abort a commit when when e.g translators tried to commit malformed translation files.. but it stayed an idea, too many things could go wrong.

> I imagine the idea is that all the developers will see the flag, then look at the diff and check for any possible security leaks.

Yep, thats why it was included in the notifications that are sent to kde-cvs, to get an overview when possibly new unsafe code is committed. I've made that as a quick "just for fun" hack, we'll see in the future if it really helps.

> Are the flagged commits sent elsewhere to be checked?

No, currently not. a local procmail / kmail filter helps :)

What fixes have been going into the cvs repository? Here is an example:

CVS commit by mueller: 

properly escape clipboard contents to avoid selecting shell-like text
actually executes it right away


  M +16 -16    klipperrc.desktop   1.82
  M +23 -26    urlgrabber.cpp   1.35
  M +2 -5      urlgrabber.h   1.15

'Many eyes' looking at code ferrets out bugs, as this commit shows:

CVS commit by lunakl: 

Max size limit for sscanf().
Also removed useless casts, the second of them hiding a bug.


  M +2 -2      app.cpp   1.53 [POSSIBLY UNSAFE]


--- kdegames/kreversi/app.cpp:1.52      Thu Sep 12 00:47:40 2002
@@ -828,8 +828,8 @@
       highscore.resize(i+1);
 
       HighScore hs;
-      sscanf((const char *)e.utf8(), "%s %d %d %d %f %ld",
-            (char *)&hs.name, &hs.color, &hs.winner,
+      sscanf( e.utf8(), "%31s %d %d %d %f %ld",
+            hs.name, &hs.color, &hs.winner,
             &hs.loser, &hs.rating, &hs.date);
       highscore[i] = hs;
     } else

In response to Lubos Lunak adding a size check to scanf, Oswald Buddenhagen responded:

> but %s in scanf() without a size limit is simply baaaaad.
> 
yeah, the kernel could be attempting to crack your box. :))))

>       f = fopen("/proc/apm", "r");
> +     s = fscanf(f, "%255s %d.%d %x %x %x %x %d%% %d %s\n",
 
seriously, if you can prove that it's poinless to add additional safety,
then don't bother to make the code less readable. maybe add a comment.

Waldo Bastian made this comment in response:

The problem is that code tends to be copied&pasted, so to promote best 
practice it is essential that all code in KDE CVS does indeed follow best 
practice. That way we have hopefully less to do when we import a new 
application in KDE CVS.

Dirk Mueller committed a patch to secure a snprintf call. Oswald Buddenhagen responded:

> it links libkdefakes so this is safer..
> 
everything (except arts and qt) links kdefakes. still, our version is
only used if no other version is present at all.

anyway, i repeat that we should not workaround snprintf breakages of
any systems nobody uses (and kde probably does not even built on).
snprintf is standardized by ISO/IEC 9899:1999. anything behaving
differently is simply broken and i see no reason to make our code less
readable and efficient because of it. add a configure check that simply
bails on failure and things are good (i think any affected person should
be happy to be told that his c lib is insecure and should be upgraded,
as kde is certainly not the only affected package).
 
> +    vsnprintf(&buffer[strlen(buffer)], sizeof(buffer)-strlen(buffer)-1, msg, args);
>  
... and consequently the "-1" should go away.

To avoid this potential insecurity, Dirk Mueller suggested:

Write a configure check and commit it. 

Oswald Buddenhagen enclosed a patch to acinclude.m4.in, with a comment

here we go ...
the test checks whether literal strings, replaced strings and replaced
integers are correctly truncated. this should be paranoid enough.
ok?

And again:

heh, that was my test whether it actually works. too bad i forgot to
revert it. :}
attached is a corrected and even more paranoid version.

Attempting o prevent shell commands from being inadvertantly executed caused some problems in Quanta. A security fix; execution of command line type plugins fixed. created an interesting situation. Most of us use ~/ for our home directory, which is expanded by the shell to /home/derek/ in my case. Andras Mantia, who wrote the original 'fix', realized he had a problem:

> >    QString args = arguments();
> > +  if (!args.isEmpty())
> > +     args = KProcess::quote(args);
>
> Are you sure that is correct? The name suggests that you can have multiple
> arguments (like what?) but if you quote them they will be interpreted as a
> single argument.
>
> Cheers,
> Waldo

Yes, you are right, and it breaks when you specify multiple arguments, but it 
does not always work even with one argument! KProcess::quote("~/dir/foo") 
returns  '~/dir/foo', which is not good for apps. I've tried it with kompare 
and cat.

andris@stein:~/cvs-developement/head/quanta> cat '~/.kderc'
cat: ~/.kderc: No such file or directory

Is this the desired behaviour of KProcess:quote()?

Waldo Bastian continued:

Yes, ~ is interpreted by the shell and expanded to your home dir, pretty much 
like ${HOME}. Quoting it disables that.

If such arguments are provided by the user and constructs like ~/.kderc are 
still supposed to work, then I would just leave it unquoted. 

The 'robot' that parsed the commits sometimes made mistakes, as this message shows:

LOL (at the "possibly unsafe" parser)
/**
 * KoContextHelpAction provides a easy to use context help system.
 * [...]
 */

At the time of this writing (Wednesday evening) a series of strcpy() fixes were committed.



New Release Manager

Dirk Mueller announced:

Stephan Kulow is the RD for KDE 3.2+

.

RD? Release Dude. Stephan Kulow commented on the use of this term:

> > > > Did the release coordinator become a director/dictator? :-)
> > >
> > > RD= Release DUDE!
> >
> > I thought so (I actually had a look into the archive before posting) but
> > I don't find the new term common or appropriate let alone the
> > abbreviation.
>
> You must be one of those new developers then >:)
> See http://www.kde.org/release.html and look at the last modified date.

Or check here, what I would call the first official use of the term :)
http://developer.kde.org/news/weekly/1999/5/

If I remember correctly 1.1.2 was the first release where tackat was involved,
and because of this the need of a release dude arised. I was kind of managing 
the releases before that, but mhk was in fact the first one to give releases 
a time frame - and in fact I was still doing the tar ball, but only those 
extracting the tars saw the kulow/students ;-)

Someone needs to write a paper, so we get these young lads educated.
 

Bug Fixes:

Docs
Bug 51459 - Nonsense in Kandy documentation
Kaboodle
Bug 50391 - completed() signal wont be emitted on playing second file, under certain circumstances
Kdat
Bug 11637 - kdat chrashed while starting a backup
Kdelibs
Bug 51733 - strlcpy in config.h conflicts with the one in string.h on Solaris
Kget
Bug 51627 - KGet leaves .part file around after deleting a transfer in progress.
Bug 51665 - Background in system tray is always black around icon
Kmail
Bug 47511 - Cannot send mail if Default identity is blank, even if it is not the ID being used
Bug 47891 - html messages is not printed as html
Bug 49802 - vcard does not get displayed, kmail shows error instead
Bug 51396 - can not show formatted html in email displayed in a separate window
IMAP Bug 51601 - how to make kmail crash after adding a new account
Kopete
Bug 51664 - Tweaks and/or "Improvements" to the UI
Bug 51675 - Chat window wastes to much space.
Jabber Plugin Bug 51587 - handling of mail like messages.
Jabber Plugin Bug 51702 - sending empty mail-like messages
Kspread
Bug 51082 - opening an older kspread returns parser errors for all formulas
Bug 51612 - action for going up while columns selected goes down
Bug 51617 - date format conversion is not correct
Kuickshow
Bug 51610 - double click to close image and load file view broken
Kview
Bug 20021 - KView also has confusing Menu entries...
Kwin
Bug 50601 - kwin crashes when closing Xnest/rdesktop window
Quanta
Bug 51308 - two open instances of quanta 3.0 cause saving trouble
Bug 51564 - Renaming a file in Quanta doesn't change its timestamp
in the new project wizard, the Clear List button deleted the content of target folder
KControl
If you selected a langauge, and removed it, it was not possible to at it without adding a different language first.
KHtml
Fix an elusive digest authentication bug
make khtml's click event handling suck a little less. fixes http://www.futureshop.ca/
oops. crash less
Kdelibs
made konqueror show garbage in the background of the list of files just when opening the context menu (RMB click).
QT-Copy
Fix for QListView not setting the brush origin correctly.


New Features:

Kopete

Work continues on Kopete, an IM client. Martijn Klingens added support for mng movies besides pixmaps.

Kmail

Zach Rusin commented on this large commit:

CVS commit by zrusin: 

Committing the folder rework. I wanted to wait a little bit, but it fixes
for me the "crash on reply" and "forward message crash" bugs and people
on IRC have been insisting on committing it. So yeah, asynchronous message
handling in KMail is in!!! May the God of Coding have mercy on us...
 

Mark Mutz and others have been working on the KROUPWARE branch of Kmail, and have merged many of the features to head. One of the features is called sieve, and here is a draft document describing sieve scripts.

Konqueror

Lubos Lunak added this feature:

Konqueror preloading (keeping last KonqMainWindow instead of destroying it,
and keeping konqy running. to be precise).
Turned off by default for now, and no GUI for the option yet (I need to
recall how to do that ;)  ) :
konquerorrc:
[Reusing]
MaxPreloadCount=

First person with a computer slow enough to measure the speed difference
(telling me it) wins a free upgrade to the kcontrol module having the GUI option.

 

Konstruct

Stephan Binner added konstruct directories throughout the repository. Testers were requested along with an explanation:

"Konstruct" is a build toolset for installing KDE 3.1rc5 plus atm KOffice,
KDevelop and Quanta from the released source tarballs. It downloads, con-
figure, compiles and installs them with a single "make install" command.

The README and archive are available at http://konsole.kde.org/konstruct/
(probably a temporary location) or you can get it from CVS with this line:

 cvs -z3 -d:pserver:anonymous@anoncvs.kde.org:/home/kde co kdenonbeta/konstruct

I'm interested to hear opinions about "Konstruct" and which bugs it has.
 

For more information, the README in /kdenonbeta/konstruct. From the README:

It downloads defined source tarballs, checks their integrity, decompresses, patches, configures, builds and installs them. A complete KDE installation should be as easy as "cd meta/kde;make install" which with this version gives a complete KDE 3.1rc5 installation. Optionally you can install additional applications like KOffice 1.2, KDevelop 2.1.4 or Quanta 3.1 too, just run for example "cd apps/koffice;make install".

By default "Konstruct" installs to ~/kde3.1-rc5 which means you don't have to possess root privileges or risk to damage your system or affect another KDE.

"Konstruct" initiated by binner@kde.org is based on the GAR ports system by Nick Moffitt (http://www.lnx-bbc.org/garchitecture.html) and is inspired by GARNOME distribution by Jeff Waugh (http://www.gnome.org/~jdub/garnome/).

Kwin

Karol Szwed added this document to /kdebase/kwin, with this comment:

Adding a NetWM compliance document listing all NetWM defined properties and messages as per freedesktop.org. Currently the compliance level for each property is empty, but will be filled in as the NetWM audit progresses.

KLaptopDaemon

Paul Campbell added a foundation for 3.2 ACPI support in this commit:

These changes lay the foundation for 3.2 ACPI support in the laptop daemon (the current stuff is a tacky short-term fudge for the few people who run 2.5 kernels). It also adds support for people with Sony laptops - it allows them to enable the scroll wheel and middle mouse button emulation ... neither of these new tabs show up in UI unless the user has and ACPI kernel or the sonypi driver installed resp.

George Staikos also added

Native support for PowerMacs. uses /proc/pmu on Linux.

Untested with multiple batteries, and untested when the battery level approaches 0.

That which has been tested, works nicely. :)

KOffice Krita

Patrick Julien added support for scanning:

Support scanning again.
Loading/Scanning and saving huge images gives back feedback and doesn't lock up GUI.
Added preliminary status bar.
various fixes.
started work on channels and masks.
Thanks for reading CVS Weekly