From c8ff1f94e8ccc4ef6573956ac4fb26e707bd7124 Mon Sep 17 00:00:00 2001 From: Jakob Unterwurzacher Date: Sat, 8 Jul 2017 00:49:51 +0200 Subject: [PATCH] Add a canary value to FileNode Adds a uint32 value to FileNode that is initialized to an arbitrary value (CANARY_OK) in the constructor, and reset when the reference is dropped (CANARY_RELEASED, CANARY_DESTROYED). The canary is checked on each withFileNode call. Makes it much easier to trigger the bug seen in https://github.com/vgough/encfs/issues/214 . --- encfs/Context.cpp | 3 +++ encfs/FileNode.cpp | 3 +++ encfs/FileNode.h | 6 ++++++ encfs/encfs.cpp | 10 ++++++++++ 4 files changed, 22 insertions(+) diff --git a/encfs/Context.cpp b/encfs/Context.cpp index eed9f57..4557884 100644 --- a/encfs/Context.cpp +++ b/encfs/Context.cpp @@ -120,10 +120,13 @@ void EncFS_Context::eraseNode(const char *path, FileNode *pl) { FileMap::iterator it = openFiles.find(std::string(path)); rAssert(it != openFiles.end()); + auto fn = it->second.front(); + it->second.pop_front(); // if no more references to this file, remove the record all together if (it->second.empty()) { + fn->canary = CANARY_RELEASED; openFiles.erase(it); } } diff --git a/encfs/FileNode.cpp b/encfs/FileNode.cpp index dea5a1a..39277d7 100644 --- a/encfs/FileNode.cpp +++ b/encfs/FileNode.cpp @@ -58,6 +58,8 @@ FileNode::FileNode(DirNode *parent_, const FSConfigPtr &cfg, Lock _lock(mutex); + this->canary = CANARY_OK; + this->_pname = plaintextName_; this->_cname = cipherName_; this->parent = parent_; @@ -76,6 +78,7 @@ FileNode::~FileNode() { // FileNode mutex should be locked before the destructor is called // pthread_mutex_lock( &mutex ); + canary = CANARY_DESTROYED; _pname.assign(_pname.length(), '\0'); _cname.assign(_cname.length(), '\0'); io.reset(); diff --git a/encfs/FileNode.h b/encfs/FileNode.h index 6662bf1..017cbee 100644 --- a/encfs/FileNode.h +++ b/encfs/FileNode.h @@ -33,6 +33,10 @@ #include "FileUtils.h" #include "encfs.h" +#define CANARY_OK 0x46040975 +#define CANARY_RELEASED 0x70c5610d +#define CANARY_DESTROYED 0x52cdad90 + namespace encfs { class Cipher; @@ -45,6 +49,8 @@ class FileNode { const char *cipherName); ~FileNode(); + uint32_t canary; + const char *plaintextName() const; const char *cipherName() const; diff --git a/encfs/encfs.cpp b/encfs/encfs.cpp index 3ff7355..029e83f 100644 --- a/encfs/encfs.cpp +++ b/encfs/encfs.cpp @@ -125,6 +125,16 @@ static int withFileNode(const char *opName, const char *path, auto do_op = [&FSRoot, opName, &op](FileNode *fnode) { rAssert(fnode != nullptr); + if(fnode->canary != CANARY_OK) { + if(fnode->canary == CANARY_RELEASED) { + RLOG(ERROR) << "canary=CANARY_RELEASED. File node accessed after it was released."; + } else if(fnode->canary == CANARY_DESTROYED) { + RLOG(ERROR) << "canary=CANARY_DESTROYED. File node accessed after it was destroyed."; + } else { + RLOG(ERROR) << "canary=0x" << std::hex << fnode->canary << ". Corruption?"; + } + throw Error("dead canary"); + } VLOG(1) << "op: " << opName << " : " << fnode->cipherName(); // check that we're not recursing into the mount point itself