But the issue here was not the command line. It was not handling the interface properly. Somebody wrote a 5 minute hack to handle security functionality and probably didn't even read the application source to look for proper use, much less version checking. The same thing happens with APIs.
And null terminated strings have worked for 40 years, that doesn't have anything to do with this.
Unless you've got a password type and a regular string type, how's the compiler to know you've swapped the password with a config string?
Joel on Software wrote an interesting article that's related to this, oddly about Hungarian notation.
His point was that *lwistrp = "foo" tells you meaningless stuff (lwip and str) about your data. That's what the compiler is supposed to track. But 'u_pw' and 's_pw' (unsafe/safe) for pre and post-processed user-input tells you something meaningful the compiler has a harder time tracking. (Not impossible with taint-tracking, but ...)
The relevance is that types tend to be based on the implementation details instead of the higher-level facts. For this sort of function to be actually safe, not just compiler-safe, it would need higher-level types and to be compiled into your language (not called via an opaque FFI) to check them.
It's not THAT bad. That's why everybody is still using it. But there are many better options, since the CLI, eg. people have created distributable libraries, and network APIs. Yet, changing APIs will break any of them.
A library, with an exposed pubic API that a compiler will, y'know, refuse to compile against when the expected arguments to a call you depend on are completely changed?
Depending on blind CLI calls that silently fail is about the least safe option possible.
The right thing IMHO would be to use pipes & stdin/stdout/stderr, but with a well-defined machine interface.
What you need is a kernel that exposes programs as a file system like plan9
- Request-reply -- Acknowledge stuff. Especially when it's security relevant
- For security relevant stuff, don't just silently fail or fall back to some idiot default. Eg. say a command message for setting a password doesn't contain the password field -- then fail the request, don't assume an empty password.
(Ie. be liberal about what you accept but not too liberal)
This looks like a developer mistaking a command line interface for an API.
Unless an (interactive) CL interface is explicitly marked as being an API, and documented in such a way, and regression tests exist to make sure the interface remains backwards-compatible - you should never program against it. Ask for a proper API to access.
(edit: formatting)
In one side, a CLI offers a "better" way of using the product (as with a library it's easier to misuse something)
On another, issues like this
(But I guess in this specific case the issue is that encfs is not doing a minimal validity check of the password being provided)
It would be good if CLIs would/could be more consistent, but in the Unix/Gnu options world, probably it won't
I'm going to do some refactoring now
If I create a small utility "just for fun" and someone includes it in a major distro and then nobody (including myself) touches it for 10 years, who is to blame if there are security (or any other) issues with the software?
Once the bug is found a ticket gets filed for it, the security teams handle it and life moves on. If the software instead isn't deemed necessary a recommendation is made to remove it from the distribution instead.
A better question is who will fix it and/or whether the maintainer should remove the package. I think such a package should be removed ASAP unless someone fixes the problem within a short amount of time.
I think two Debian policies interact poorly: heavily modifying upstream sofware, and not doing their own security review. To my mind the Debian SSH key vulnerability was a predictable result of this policy combination (which Debian did not see fit to change AIUI) and similar issues are to be expected from Debian going forward.
Does any distro (outside OpenBSD) have a continuous process of re-evaluating included packages?
Do you know whether OpenBSD strives for consistency and throws away stuff when there is a better alternative?
- All ICs having direct (unlimited) access to memory and/or computation (CPU/DMA/GPU/BIOS)
- Boot loader
- Operating system
- Encryption algorithm
- Encryption implementation
- User-program used to interface with encryption implementation
- Input/output peripherals used for encryption and display
- Probably a dozen things I'm forgetting here
But that's only the system itself. Obviously, you will need to formalize the relationship of the system with the environment. This means: side-channel attacks, such as timing, electro-magnetic radiation, sound and temperature.
Finally, you need to formalize the relationship of the user and whatever is encrypted. The user needs to have an understanding of the limitations of the encrypted channel, the used keys and the algorithms involved: what is the availability, the loss of entropy to attackers and the limitations to what can safely be encrypted?
For some trivial tests against a library or application, you use test vectors calculated by hand. Every crypto algorithm has some published vectors out there. That would discover this flaw, but is not enough by a huge margin. In practice, you don't test crypto software beyond the trivial, you review it and proof correctness against the attacks you know.
For this to fail one of the following must hold: 1 you made a mistake in your decryption that matches a mistake in the encryption.
2 The encryption scheme allows incorrect cyphertext to decrypt to correct plaintext
3 The encryption software writes more than the supposed cyphertext.
1 requires a mistake on your end, so is hard for malicious entities to abuse. 2 is a feature of the cypher, as such can be defended against in design. 3 can be defended against by observing the encryption software (look for all disk modification and network activity).
in terms of automated tests that could have caught this error:
1). you can create a second implementation and check that it properly decrypts the output from the first implementation and check the first implementation will properly decrypt the output from the second implementation.
2) you can create output from what you believe is a known good program. then write an automated test that would decrypt this output. then write an automated test that would perform an encryption and then a decryption. this would catch a problem where the encryption and decryption routines switch to using a hardcoded string because of a bug.
warning: 2) only works if the bug doesn't exist when you first write the test. :)
3). you allow the random values that making testing encryption difficult to be stubbed out then you can just encrypt plaintext and check it for equality with the ciphertext.
---->
of course there is a lot more issues you should be testing for and google recently released a framework that checks for common implementation errors:
For those who want alternative to cryptkeeper,there is SiriKali[1], next version will be released on Feb 1st and it will have support for OSX.
execlp ("encfs", "encfs", "-S", crypt_dir, mount_dir, NULL);
That line of code also has a bug as its wrong to pass NULL in C++ to a variadic function. IMHO,usage of NULL in C++ should be strongly discouraged in all cases.To quote the linux man page, "The list of arguments must be terminated by a NULL pointer...".
For variadic C functions, you MUST use 0 (or NULL, which is the same as 0). Passing nullptr will case abort() to be called (as is the case whenever you pass a C++ class into a variadic function).
I understand their similarity in terms of NULL's equivalence to memory address 0 and boolean false, but I'm increasingly aware that there's some hidden behavior.
Does NULL strictly resolve to '(void * )0', or can it be influenced by anything else?
Also, does it behave differently in C vs. C++? I suspect it might.
EDIT: Someone else just replied close to this thread chain just said it's defined in C++ as 0 without '(void * )'. If anyone can expand on this that would be awesome.
You can't pass nullptr to a variadic function. It causes a run-time call to abort. You can pass 0, or NULL (which is just a macro defined to 0).
From the execlp manpage: "The list of arguments must be terminated by a null pointer, and, since these are variadic functions, this pointer must be cast (char *) NULL."
Using bare "NULL" or "nullptr" in this context is wrong (although you might get away with the former on many platforms).
Is it a vulnerability if the product wasn't even working in the first place?
He figured out from the source code that the password was "p", but for many people it'd just mean important data lost.
I just wonder if thousands, or hundreds, or at least a dozen eyeballs actually watched this source. I suspect 1-2 pairs of eyeballs would detect the problem if a proper code review was done.
Even better, if an automated test existed for this pretty critical piece of code (passing the password around), thousands of eyeballs won't even be needed that badly.
I suspect that just too few people actually use this piece of software. If a large org used it, they probably would have tested this, or even audited the code.
Anyway, it's great that everyone has access to the code, so the bug was detected by a user. For closed-source software, a ticket like this might linger longer for a developer's attention.
My point was the eyeballs argument is often made as a claim about the absence of bugs at the present time.
When in fact it simply means "it is possible for people to look at it and find and fix bugs" no more no less.
The mere ability does not magically translate to security. And the ability does not automatically imply that it is being done.
Very hard to test for as there weren't explicit dependencies and no amount of 'don't count on this output staying the same' warning messages helped.
As the comment in the code says, it's for setting the pre-configured "Paranoia" mode (AES, PBKDF2, IV-chaining, etc) in encfs.
Bad that it's not checking any results whatsoever when sending stuff..