Niels, On 12/12/2012 09:04 AM, Niels Thykier wrote: > In regards to the actual changes, I suspect they are flawed in the > "error"-path, see "cracklib2.review".
Doh! You are absolutely right. Nice catch, thanks. I can confirm that I (still) get the correct error message with your suggested changes (from python, in case of a missing dict, that is): > exception thrown: [Errno 2] No such file or directory: > '/var/cache/cracklib/cracklib_dict.pwd' That is on my usual work machine (amd64) as well as on a kirkwood (armv5tel) (both on Debian wheezy). (I'm surprised it worked before. It certainly did work as expected on the amd64 system... extensive use of compiler magic, I guess). The modified patch is attached, as I tested it. I'm sorry for not getting this correct the first time. Regards Markus Wanner
Subject: add a safer check variant Author: Markus Wanner <[email protected]> Bug-Debian: http://bugs.debian.org/682735 --- a/lib/fascist.c +++ b/lib/fascist.c @@ -879,6 +879,48 @@ return res; } +/* This Debian specific method is a work-around for Debian #682735. Please + do not rely on it being available in future verisons of cracklib2. */ +int +__DEBIAN_SPECIFIC__SafeFascistCheck(password, path, errstr) + const char *password; + const char *path; + char **errstr; +{ + PWDICT *pwp; + char pwtrunced[STRINGSIZE]; + + /* If passed null for the path, use a compiled-in default */ + if ( ! path ) + { + path = DEFAULT_CRACKLIB_DICT; + } + + /* security problem: assume we may have been given a really long + password (buffer attack) and so truncate it to a workable size; + try to define workable size as something from which we cannot + extend a buffer beyond its limits in the rest of the code */ + + strncpy(pwtrunced, password, TRUNCSTRINGSIZE); + pwtrunced[TRUNCSTRINGSIZE - 1] = '\0'; /* enforce */ + + /* perhaps someone should put something here to check if password + is really long and syslog() a message denoting buffer attacks? */ + + if (!(pwp = PWOpen(path, "r"))) + { + return 0; + } + + /* sure seems like we should close the database, since we're only likely to check one password */ + *errstr = FascistLook(pwp, pwtrunced); + + PWClose(pwp); + pwp = (PWDICT *)0; + + return 1; +} + const char * GetDefaultCracklibDict() { --- a/python/_cracklibmodule.c +++ b/python/_cracklibmodule.c @@ -42,6 +42,7 @@ #ifdef HAVE_LIBINTL_H #include <libintl.h> #endif +#include <errno.h> #ifdef HAVE_PTHREAD_H static pthread_mutex_t cracklib_mutex = PTHREAD_MUTEX_INITIALIZER; @@ -74,7 +75,8 @@ { char *candidate, *dict; char *defaultdict = NULL; - const char *result; + int result; + char *errmsg; struct stat st; char *keywords[] = {"pw", "dictpath", NULL}; char *dictfile; @@ -148,7 +150,8 @@ #endif LOCK(); - result = FascistCheck(candidate, dict ? dict : defaultdict); + result = __DEBIAN_SPECIFIC__SafeFascistCheck(candidate, + dict ? dict : defaultdict, &errmsg); UNLOCK(); if (defaultdict != NULL) @@ -156,11 +159,26 @@ free(defaultdict); } - if (result != NULL) + if (result) { - PyErr_SetString(PyExc_ValueError, result); - return NULL; + if (errmsg != NULL) + { + PyErr_SetString(PyExc_ValueError, errmsg); + return NULL; + } + } else { + if (errno == 0) + { + PyErr_SetString(PyExc_RuntimeError, "Unable to read cracklib dictionary."); + return NULL; + } + else + { + PyErr_SetFromErrnoWithFilename(PyExc_ValueError, "/var/cache/cracklib_dict.*"); + return NULL; + } } + return Py_BuildValue("s", candidate); } --- a/lib/crack.h +++ b/lib/crack.h @@ -15,6 +15,14 @@ extern const char *FascistCheck(const char *pw, const char *dictpath); +/* This Debian specific method is a work-around for Debian #682735. Please + do not rely on it being available in future verisons of cracklib2. + Returns 1 (true) for success and 0 (false) in case an error occurred + opening or reading the dictionary. In the later case, please check + errno. */ +extern int __DEBIAN_SPECIFIC__SafeFascistCheck(const char *pw, + const char *dictpath, char **errmsg); + /* This function returns the compiled in value for DEFAULT_CRACKLIB_DICT. */ extern const char *GetDefaultCracklibDict(void); --- a/lib/packlib.c +++ b/lib/packlib.c @@ -16,6 +16,7 @@ #ifdef HAVE_STDINT_H #include <stdint.h> #endif +#include <errno.h> #include "packer.h" static const char vers_id[] = "packlib.c : v2.3p2 Alec Muffett 18 May 1993"; @@ -156,6 +157,7 @@ if (!fread((char *) &pdesc.header, sizeof(pdesc.header), 1, ifp)) { fprintf(stderr, "%s: error reading header\n", prefix); + errno = 0; pdesc.header.pih_magic = 0; fclose(ifp); @@ -179,6 +181,7 @@ if (!fread((char *) &pdesc64.header, sizeof(pdesc64.header), 1, ifp)) { fprintf(stderr, "%s: error reading header\n", prefix); + errno = 0; pdesc.header.pih_magic = 0; fclose(ifp); @@ -198,6 +201,7 @@ { /* nope, not "64-bit" after all */ fprintf(stderr, "%s: error reading header\n", prefix); + errno = 0; pdesc.header.pih_magic = 0; fclose(ifp); @@ -224,6 +228,7 @@ if (pdesc.header.pih_magic != PIH_MAGIC) { fprintf(stderr, "%s: magic mismatch\n", prefix); + errno = 0; pdesc.header.pih_magic = 0; fclose(ifp); @@ -244,6 +249,7 @@ if (pdesc.header.pih_numwords < 1) { fprintf(stderr, "%s: invalid word count\n", prefix); + errno = 0; pdesc.header.pih_magic = 0; fclose(ifp); @@ -263,6 +269,7 @@ if (pdesc.header.pih_blocklen != NUMWORDS) { fprintf(stderr, "%s: size mismatch\n", prefix); + errno = 0; pdesc.header.pih_magic = 0; fclose(ifp);
