[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-devel
Subject:    Re: nspluginviewer still crashes
From:       Waldo Bastian <bastian () kde ! org>
Date:       2001-08-11 1:23:28
[Download RAW message or body]

On Friday 10 August 2001 05:43 pm, Dirk Mueller wrote:
> On Fre, 10 Aug 2001, Waldo Bastian wrote:
> > Someone replaced the working version with some BSD crap it seems.
>
> I guess you mean me as I'm suddenly CC'ed. I fail to see where mkstemps is
> "BSD crap". could you explain please ? 

I refer to the crap that was previously between #ifdef __FreeBSD__ , also 
known as "horrible broken code" and that you used as inspiration for the 
current code, instead of using the part that was in the #else branch 
which actually worked. 

> What I did is fixing the horrible
> broken code that was #ifdef'ed for *BSD before and caused regular crashes
> of all kind (as it buffer-overflowed, leaked and feeded a system call with
> invalid input). I understood at that point why *BSD'ers where complaining
> about the lack of stability in konqueror/KDE.

If the "BSD-maintainer" insist on adding "horrible broken code" for his 
platform then that is his problem, but that doesn't mean it should be used 
for other platforms as well.

> Anyway, your patch seem to respect all the loopholes about tmp-file attacks
> I know of, but still using a standard function that has been reviewed half
> a million times is imho safer than reinventing the wheel.

But it is broken by design. I don't find "may not contain 'X'" an acceptable 
limitation in an API.

Well, that's not completely true, it seem to be mostly broken by 
implementation, something like this should fix it:

-   mTmpName = filePrefix+QString("XXXXXX")+fileExtension;
-   QCString nme = QFile::encodeName(mTmpName);
-   if((mFd = mkstemps(nme.data(), nme.length()-nme.findRev('X')-1)) < 0)
+   QCString ext = QFile::encodeName(fileExtension);
+   QCString nme = QFile::encodeName(filePrefix) + "XXXXXX" + ext;
+   if((mFd = mkstemps(nme.data(), ext.length())) < 0)

> Plus its still a major fault of nspluginviewer if it uses website-delivered
> data unchecked for local file creation, because it could still contain
> backreferences, special shell characters, whatever you can think of.

KTempFile handles those just fine.

Cheers,
Waldo
-- 
KDE 2.2: We deliver.
 
>> Visit http://master.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

[prev in list] [next in list] [prev in thread] [next in thread]