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 <[email protected]>
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.

The advisory is posted here:

https://www.mhcomputing.net/redis-advisory-2013.txt

Thanks,
Matthew Hall