Opened 9 years ago

Closed 3 years ago

#3370 closed defect (duplicate)

Race condition - file system access in Tor (toctou bugs)

Reported by: runa Owned by:
Priority: Medium Milestone: Tor: unspecified
Component: Core Tor/Tor Version: Tor: 0.2.2.27-beta
Severity: Normal Keywords: tor-client
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

I have scanned tor-0.2.2.27-beta with Fortify, which picked up a few issues in the "Race Condition: File System Access" category. I believe we have discussed these types of issues before, but I couldn't find a bug where they were all listed.

config.c:4723 - regarding the window of time between the call to read_file_to_str() and rename().

config.c:5124 - regarding the window of time between the call to read_file_to_str() and or_state_save_broken().

config.c:5138 - regarding the window of time between the call to read_file_to_str() and or_state_save_broken().

routerlist.c:835 - regarding the window of time between the call to tor_mmap_file() and remove_file_if_very_old().

I don't think we need to worry too much about these issues, but maybe we should be sure to check the files before we read/write/execute/move?

Child Tickets

Change History (4)

comment:1 Changed 9 years ago by nickm

Milestone: Tor: unspecified

We should fix the part of these that we can for cleanliness. Most of the "if file_status()==FN_FILE then read_file_to_string" checks are silly: they could just be read_file_to_string with an appropriate response to errors.

In fact, most file_status() checks should be considered suspect. Generally, instead of "may I do X with this file" then trying it, we should just try to do X and see if we get an error.

comment:2 Changed 8 years ago by nickm

Keywords: tor-client added

comment:3 Changed 8 years ago by nickm

Component: Tor ClientTor

comment:4 Changed 3 years ago by nickm

Resolution: duplicate
Severity: Normal
Status: newclosed
Summary: Race condition - file system access in TorRace condition - file system access in Tor (toctou bugs)

closing this ticket here, since coverity tracs all of these as toctou errors.

(These are not actual bugs in this case, and some are unavoidable given the lack of portable unlinkat(2) calls and other stuff.)

Note: See TracTickets for help on using tickets.