Bug#656154: opencity: unsafely overwrites opencity.save

[Francesco Poli (wintermute)]

Package: opencity
Version: 0.0.6.4stable-1
Severity: important

Hi there!

First of all, thanks a lot for maintaining OpenCity in Debian!

I think it's a really nice game, with a great potential.
I especially like its truly 3D approach to city simulation,
and the fact that it's not a clone of any proprietary game,
even though it clearly belongs to a well-defined game genre.

I hope the upstream developers will continue to develop it,
since it may become a very entertaining game, if some rough edges
get smoothed out!


All that said, I would like to point out a feature that seems
to be implemented in an unsafe manner.
The save button lets the user save the city.
As the documentation [1] on the official web site states:

| Currently, OpenCity handles only one save slot. It will overwrite
| any previously saved city.

[1] http://www.opencity.info/en/TutorialMainMenu.html

OK, this is a limitation (I will file a separate wishlist bug about it),
but that's not the point.
I can live with a single save slot, which corresponds to
~/.opencity/opencity.save

The problem is: it seems to me that, when the save button is pressed,
the program opens the save file ~/.opencity/opencity.save in
write mode and begins writing the current city status to it.

This implies that it exists a short time span, where the previously
saved city is no longer stored, but the current city is not yet written
to the save file. If something goes wrong during this short time span,
the user loses his/her city!

I haven't looked into the source code, hence I am not sure of the
above explanation, but I am sure that something similar may happen,
as I personally experienced the issue!
I was saving my city, when the program segfaulted (yes, I experienced
a good number of segfaults, but I haven't yet found an easily
reproducible one...): after that, I was left *without* any usable save
file. I therefore lost my city!   :-(


I think a safer save strategy should be implemented: maybe the
program should save to a temporary file, and then, *only after*
a reasonable integrity check, rename the temporary file to
~/.opencity/opencity.save

Or otherwise, the program could first copy ~/.opencity/opencity.save
to ~/.opencity/opencity.save.bak and, *after that* open
~/.opencity/opencity.save in write mode and write to it.


Please fix this flaw and/or forward this bug report upstream.

Thanks for your time!


-- System Information:
Debian Release: wheezy/sid
  APT prefers testing
  APT policy: (800, 'testing'), (500, 'unstable')
Architecture: amd64 (x86_64)

Kernel: Linux 3.1.0-1-amd64 (SMP w/2 CPU cores)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/bash

Versions of packages opencity depends on:
ii  libc6                     2.13-24
ii  libgcc1                   1:4.6.2-11
ii  libgl1-mesa-glx [libgl1]  7.11.2-1
ii  libglu1-mesa [libglu1]    7.11.2-1
ii  libpng12-0                1.2.46-3
ii  libsdl-image1.2           1.2.10-3
ii  libsdl-mixer1.2           1.2.11-7
ii  libsdl-net1.2             1.2.7-5
ii  libsdl1.2debian           1.2.14-6.4
ii  libstdc++6                4.6.2-11
ii  libtinyxml2.6.2           2.6.2-1
ii  opencity-data             0.0.6.4stable-1

opencity recommends no packages.

opencity suggests no packages.

-- no debconf information