[patch] Fix g10/exec.c (make_tempdir) [_WIN32] not properly handling GetTempPath

[Israel G. Lugo]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
 
David Shaw wrote:
> So MAX_PATH is not, in fact, the maximum path on Windows?  Wow,
> that's a good idea.  Is there a MAX_PATH_WE_REALLY_MEAN_IT or
> similar that I can use here?  What is the real maximum path?
The GetTempPath reference says nothing of there being a limit of any
sort, in particular of the buffer being limited to MAX_PATH bytes. It
is certainly perfectly possible to set an environment variable longer
than MAX_PATH (the API reference mentions the limit for an environment
variable being 32768 including '\0'). I don't know whether a large
$TMP will work or not in current versions of Windows. However, I say
again: why impose an unnecessary limit of our own when we can
elegantly deal with arbitrarily long data?

There was a time when 8+3 characters for a filename seemed to some
"everything you would ever need". There was a time when no one would
think of using more than 2 digits to store the year - that turned out
nicely.

Then there's the other side of the coin: when $TMP is a small string,
why waste space allocating a huge MAX_PATH array? You can easily
allocate just the necessary amount of space, no more, no less.

> To what end?  We could write a dozen fallbacks: "well, if
> GetTempPath fails, we'll figure out the localization of the
> particular system and try a few directories, if that fails we'll
> figure out what version of Windows it is and try a few
> directories..."
I was hardly suggesting that... I had mentioned the
GetWindowsDirectory API, and I had also mentioned that it might also
make sense to just fallback to the hardcoded path since GetTempPath
does try to obtain the Windows directory as well, as long as this was
done in case of error and not just because a $TMP path was larger than
some arbitrary limit.

> Give me a number and I'll raise the size, but layers of fallbacks
> strikes me as unnecessary complication in a relatively unimportant
> place in the code.
Why have a number at all? The solution I had proposed would have dealt
with any size in an efficient manner, without wasting memory
allocating 1,000,000 bytes "just to be sure" or failing because "42 is
more than enough for anyone".

> Note also that this incorrect code has been in GnuPG for around 4
> years now, and nobody noticed, meaning nobody used a temporary
> directory path larger than 256 bytes...
Uhh... I'm sorry for trying to help, then? I don't see the need to
become so defensive.

I very much enjoy the GnuPG program, and I very much appreciate all
the effort made by the GnuPG team. I saw something which could be
improved, and I wanted to give something back and contribute, as a
fellow developer, in the spirit of open source. I saw a hardcoded 256
limit that could easily be gotten rid of, a possibly large problem
(which might even be a security risk) as there was no check being made
as to whether or not GetTempPath succeeded before using tmp (setting
$TMP to something > 256 bytes would at the very least DoS the program
as tmp would not be initialized, which could conceivably even bring
worse consequences later on). I thought "hey why not help and give
something back" so I made a patch, reported the problem and offered a
solution.

Quite frankly, the reaction I've gotten makes me feel that I should
have said nothing in the first place...

> This is not a "Windows=Bad, Unix=Good" thing!  Both Windows and
> Unix provide an API for getting a temporary directory.  If that API
> fails, then that's it, on whatever platform.  In the Unix code, if
> you set $TMPDIR to garbage, then it won't work.  In neither case do
> I think that trying many different directories to try and find a
> "good" one is advisable.
The c:\windows\temp thing wasn't even my main point... You will notice
I did not address that in the patch I sent. My point was fixing a
quite real problem (GetTempPath's result was not being checked and tmp
could be used with complete garbage inside if, say, $TMP was > 256,
for example) and improving something which could be improved (loosing
the hardcoded malloc and switching to a flexible, non-limited and
non-wasting solution).

I wanted to to offer my help in a project which I find worthy and
noble, to contribute and engage in a friendly technical discussion
with fellow developers; not to argue with them.

Regards,
Israel
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.3 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
 
iD8DBQFEdPiPgn9msFnMGkcRAoUcAJ9DFkxIl3UHQh8xCLYzi5DfSnkBkACgrOqc
kvZw63Bp9d2KcTbPCBfYx2g=
=Y88O
-----END PGP SIGNATURE-----