full-disclosure - temporary file creation vulnerability in Redis

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20140222013327.GA28464@mhcomputing.net>
Date: Fri, 21 Feb 2014 17:33:27 -0800
From: Matthew Hall <mhall@...omputing.net>
To: full-disclosure@...ts.grok.org.uk, submissions@...ketstormsecurity.com
Subject: temporary file creation vulnerability in Redis

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

_______________________________________________
Full-Disclosure - We believe in it.
Charter: http://lists.grok.org.uk/full-disclosure-charter.html
Hosted and sponsored by Secunia - http://secunia.com/