oss-security - Fwd: temporary file creation vulnerability in Redis

Follow @Openwall on Twitter for new release announcements and other news

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20140222203535.GA32311@mhcomputing.net>
Date: Sat, 22 Feb 2014 12:35:35 -0800
From: Matthew Hall <mhall@...omputing.net>
To: oss-security@...ts.openwall.com
Subject: Fwd: temporary file creation vulnerability in Redis

Hello,

Could someone please assign me a CVE for the below Redis vulnerability?

Thanks,
Matthew Hall

----- Forwarded message from Matthew Hall <mhall@...omputing.net> -----

Date: Fri, 21 Feb 2014 17:33:27 -0800
From: Matthew Hall <mhall@...omputing.net>
To: full-disclosure@...ts.grok.org.uk
Subject: temporary file creation vulnerability in Redis
User-Agent: Mutt/1.5.21 (2010-09-15)

See also: https://github.com/antirez/redis/issues/1560 .

I have been trying to reach the Redis maintainers since 2013-09-13 regarding 
this report, but I could not find a good security contact for Redis, and the 
lead maintainer, Salvatore Sanfilippo <antirez@...il.com> is not replying to 
my private report to him about the issue and his opinion of it. I also 
contacted US-CERT for help and they could not reach anyone by 2014-01-24.

Therefore I would like to encourage the Redis team to be more 
security-friendly and establish some contact procedures on their website. 
Given how many places this software is now being used these days, I think it 
is very critical to make these changes before someone finds something more 
serious than the one I could spot.

I think I might have discovered a security vulnerability in Redis 2.6.16. This 
code is from the function int rdbSave(char *filename) in rdb.c:

   630  int rdbSave(char *filename) {
   631      dictIterator *di =3D NULL;
   632      dictEntry *de;
   633      char tmpfile[256];
   634      char magic[10];
   635      int j;
   636      long long now =3D mstime();
   637      FILE *fp;
   638      rio rdb;
   639      uint64_t cksum;
   640
   641      snprintf(tmpfile,256,"temp-%d.rdb", (int) getpid());
   642      fp =3D fopen(tmpfile,"w");
   643      if (!fp) {
   644          redisLog(REDIS_WARNING, "Failed opening .rdb for saving: %s",
   645              strerror(errno));
   646          return REDIS_ERR;
   647      }
...
   692      /* Make sure data will not remain on the OS's output buffers */
   693      fflush(fp);
   694      fsync(fileno(fp));
   695      fclose(fp);
   696
   697      /* Use RENAME to make sure the DB file is changed atomically only
   698       * if the generate DB file is ok. */
   699      if (rename(tmpfile,filename) =3D=3D -1) {
   700          redisLog(REDIS_WARNING,"Error moving temp DB file on the final destination: %s", strerror(errno));
   701          unlink(tmpfile);
   702          return REDIS_ERR;
   703      }

In line 641, the function does not use a security temporary file creation 
routine such as mkstemp. This is vulnerable to a wide range of attacks which 
could result in overwriting (in line 693-695) and unlinking (in line 701) any 
file / hard link / symlink placed in temp-PID.rdb by an attacker.

https://www.owasp.org/index.php/Improper_temp_file_opening
https://www.owasp.org/index.php/Insecure_Temporary_File

The code should be creating the temporary file using some kind of safe 
function like mkstemp, O_EXCL open, etc. instead of just using a PID value 
which does not have enough entropy and protection from race conditions. It 
should also be sure it has set the CWD of itself to a known-safe location that 
should have permissions which are only open to the redis daemon / redis user 
and not to other users or processes.

Thanks,
Matthew Hall

----- End forwarded message -----

Please check out the Open Source Software Security Wiki, which is counterpart to this mailing list.

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.