Angband.oook.cz
Angband.oook.cz
AboutVariantsLadderForumCompetitionComicScreenshotsFunniesLinks

Go Back   Angband Forums > Angband > Development

Reply
 
Thread Tools Display Modes
Old July 7, 2011, 08:28   #11
Blue Baron
Adept
 
Join Date: Apr 2011
Posts: 103
Blue Baron is on a distinguished road
Quote:
Originally Posted by myshkin View Post
Here is a decent summary of the potential security issues surrounding temporary file creation.
Thank you, that was interesting.

I still don't think I really understand the security issues, but I've attached are some diff files that I think acknowledges some of the issues. It uses simple random filenames, does not delete anything unless it moved the file to the filename, uses open with the create and exclusive flags, and fails if a file already exists. It doesn't use mkstemp, because I didn't want to worry about the different platforms.

If it is not sufficient, you might want to add a MODE_WRITE_TEMP and add something like the following to file_open:
Code:
    case MODE_WRITE_TEMP:
    {
        int fd = mkstemp(buf);
        if (fd < 0) {
            f->fh = NULL;
        } else {
            f->fh = fdopen(fd, "wb");
        }
    }
and use MODE_WRITE_TEMP instead of MODE_WRITE in the file_open call in savefile_save().

By the way, there are some implementations of mkstemp that could be used for the platforms that don't have it at:
ftp://ftp.math.utah.edu/u/ma/hohn/li...port/mkstemp.c
and
http://www.opensource.apple.com/sour.../src/mkstemp.c
Attached Files
File Type: zip savefile_diff.zip (1.7 KB, 97 views)
Blue Baron is offline   Reply With Quote
Old July 7, 2011, 08:32   #12
Pete Mack
Prophet
 
Join Date: Apr 2007
Location: Seattle, WA
Posts: 5,385
Donated: $40
Pete Mack is on a distinguished road
Temp file collision is only a security issue when there's a vector for malicious use. Since angband is not executing arbitrary code, there isn't one here.
Pete Mack is offline   Reply With Quote
Old July 7, 2011, 17:20   #13
Blue Baron
Adept
 
Join Date: Apr 2011
Posts: 103
Blue Baron is on a distinguished road
Quote:
Originally Posted by Pete Mack View Post
Temp file collision is only a security issue when there's a vector for malicious use. Since angband is not executing arbitrary code, there isn't one here.
If it doesn't get in the way of the game, it should be made more secure just on general principles/software practices. (you never know when a sys admin will play the game when they shouldn't on an account that they shouldn't ) I think the security worry is an attacker making a link between what the game uses for a temporary file and some important file, then the game overwriting or deleting that file when writing/deleting the temporary files.

Also, for ticket #1462, when the visuals are written while in a graphics mode, what is written of course uses that graphics mode. When the info is read back in at save file load, if the game is already using a different graphics mode, the information read will be wrong. To ignore this the way the other platforms do and avoid the crash, then you can add one line to sdl_BuildTileset() and change to:
Code:
GfxInfo *info = &GfxDesc[use_graphics];
if (!use_graphics) return (1);
This should work, but I have not tried it because I do not use the SDL port.

Lastly, could someone give me a sequence to reproduce ticket #758? I don't know how to generate the files. When I tried saving both options and visuals, with a second character in a save file, both were saved in the same file.
Blue Baron is offline   Reply With Quote
Reply


Currently Active Users Viewing This Thread: 1 (0 members and 1 guests)
 
Thread Tools
Display Modes

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off

Forum Jump

Similar Threads
Thread Thread Starter Forum Replies Last Post
Bug report: shopkeeper greetings Derakon Development 4 June 19, 2011 19:43
Bug report - negative min values in ego_item.txt jens Development 5 June 19, 2011 13:30
Bug report - money not updated jens Development 0 June 18, 2011 23:20
Bug Report - Subwindow refresh after options load dhegler Vanilla 3 March 2, 2010 19:56
Vanilla Bug Report: Everburning Lantern The G Masta Vanilla 1 November 11, 2009 18:35


All times are GMT +1. The time now is 11:04.


Powered by vBulletin® Version 3.8.11
Copyright ©2000 - 2019, vBulletin Solutions Inc.