Thread: [Passwordsafe-devel] SecString class
Popular easy-to-use and secure password manager
Brought to you by:
ronys
From: Edward E. <ed...@ya...> - 2002-03-01 00:28:21
|
Hi all, A few weeks ago, ignorant of this list, I emailed Jim Russell about a problem with the PWSafe source. I noticed the SecString class at that time was subclassing std::string, which is a big no-no since std::string has no virtual destructor. In some instances, this can lead to the SecString destructor never being called, and hence the memory buffer with the SecString contents never getting cleared. I suggested a better approach would be to write an allocator that clears memory on release, then make SecString an instantiation of the std::string template using this allocator. The beauty of this solution is you can create any STL container that clears its memory on release. I mentioned I would be writing such an allocator class myself soon for another project and would gladly provide a copy of the source. Well, as I was browsing the Crypto++ library I came across an allocator class (template, actually) written by a Denis Bider that does exactly that. On Windows platforms, it also attempts to lock the pages in memory with the VirtualLock() system call. I was contemplating adding this functionality to my allocator as well, although the MSDN information is conflicting as to whether VirtualLock actually locks the pages in memory or just hints to the OS that is should do so. Anyway, the allocator is available (no license restrictions) as part of the BSL library at http://www.bitvise.com/bsl.html I don't know this Denis Bider and I can't vouch for the quality of the code. But on first pass it looks reasonably solid. My one complaint would be the simplistic memory overwriting which merely zeroes the memory out one time, but this can easily be changed. I hope to give the code a more thorough review in the near future. At that time I will inform this list of my findings and provide and code changes I make. Thanks, Edward Elliott __________________________________________________ Do You Yahoo!? Yahoo! Greetings - Send FREE e-cards for every occasion! http://greetings.yahoo.com |
From: James C. <James@NovelTheory.com> - 2002-05-25 15:37:32
|
I've just join in, and figure that I'd throw in my two cent on the recent conversations: >> I noticed the SecString class at that time was subclassing std::string, which is a big no-no since std::string has no virtual destructor. In some instances, this can lead to the SecString destructor never being called, and hence the memory buffer with the SecString contents never getting cleared. << Actually, there is NO problem here, because there is no conceivable situation where the dtor would not be called. To prove this, you just have to look at the one (inconceivable) scenario, where it is a problem: void Func() { std::string* pStr = GetANewString(); // use pStr delete pStr; // ~std::string() called, not subclass dtor } Further, we must have the ability (and desire) to change GetANewString() (to return a subclass of std::string), but (and here's the key), we must be UNABLE to change Func(). However, considering that code, could there be any possible reason for writing that idiom, *unless* it was to use pStr polymorphically? In which case, the programmer, *expects* a subclass of string there, and so the code is broken before you touched a thing. In other words, we'd be making design decisions based on the theoretically possibility of breaking code that's already broken. Now, if we are able to change Func() (and, since we have the complete source code at our disposal, we would be able to, if such a function existed), then fixing it is trivial: class vstring : public std::string { vstring(const char* _Ptr, size_type _Count = npos) : std::string(_Ptr, _Count) {} // similarly duplicator other ctors virtual ~vstring() = 0; }; class SecString : public vstring { // etc }; void Func() { vstring* pStr = GetANewString(); // use pStr delete pStr; // subclass dtor called } -------------- The problem with the alternative you suggest of create a new instantiation of basic_string with a specific allocator, to that you lose the IS-A relationship of inheritance. For example, an existing function which took a std::string& parameter would work perfectly with a string-derived class, but not with a new basic_string type, since it would have not relationship with std::string. Truth, James Curran |
From: Jim R. <jru...@us...> - 2002-03-01 01:38:20
|
> A few weeks ago, ignorant of this list, I emailed Jim Russell about a > problem with the PWSafe source. I noticed the SecString class at that > time was subclassing std::string, which is a big no-no since > std::string > has no virtual destructor. In some instances, this can lead to the > SecString destructor never being called, and hence the memory buffer > with the SecString contents never getting cleared. I should point out to the list what I mentioned to Edward when he wrote me. First, SecString is experimental code that is *not* currently being used by the Win32 Password Safe executable. It is basically "notes to myself", and may or may not compile as is. We are looking for ways to free PWSafe from the CString class of MFC. Second, the 'string' class that I was inheriting from in my experiments is my own 'stl-like' string class, whose destructor is virtual. Okay, disclaimers out out the way, Edward makes an excellent point. Any "secure string" class that gets used here should do its security magic with its own allocator. Edward, thanks for following up on that, and I too will take a look at Denis Bider's code. The current PWSafe code (the real code that goes in the build) does multiple overwrites, but doesn't go to the kind of extremes that Peter Gutmann recommends in his excellent paper whose title I have forgotten at the moment. By the way, apologies to all for my quietness here of late. I've been a victim of the current economic climate (translation -- I'm job hunting), and I'm doing hourly consulting to pay the mortgage. I've been hacking at PWSafe, but I've let updates to the sourceforge page slide. I'm going to try to rectify that in the next few days. Jim R |