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 is not replying to
my private report to him about the issue and his opinion of it.
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 = NULL;
632 dictEntry *de;
633 char tmpfile[256];
634 char magic[10];
635 int j;
636 long long now = mstime();
637 FILE *fp;
638 rio rdb;
639 uint64_t cksum;
640
641 snprintf(tmpfile,256,"temp-%d.rdb", (int) getpid());
642 fp = 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) == -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