gregorynicholas / py-bcrypt

Automatically exported from code.google.com/p/py-bcrypt
Other
0 stars 0 forks source link

Crash in bcrypt_hashpw if py-bcrypt is compiled on Windows 7 with cygwin, gcc. #3

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?

I understand that py-bcrypt might not have been intended to be compiled under 
windows with cygwin. However, there are people who might still go ahead and do 
it and run into issues. I had some problem finding the cause and the fix and 
wanted to share it in case someone else runs into it.

1. Compile and install py-bcrypt on Windows 7 using cygwin and gcc. I have 
personally seen this happen on Windows 7, and it reportedly happens on Vista as 
well.

You will also need to add these lines to every .c/.h file in py-bcrypt to 
compile under cygwin...

typedef unsigned char u_int8_t;
typedef unsigned short u_int16_t;
typedef unsigned int u_int32_t;

2. Run this simple test program

import bcrypt
password = 'my secret password'

# next line crashes on Windows Vista / 7
hashed = bcrypt.hashpw(password, bcrypt.gensalt())

# next line should return True
print bcrypt.hashpw('my secret password', hashed) == hashed

3. Crash!!

What is the expected output? What do you see instead?
Expected is the output 'True'. Program is killed prematurely.

What version of the product are you using? On what operating system?
py-bcrypt 0.2. Windows 7

Please provide any additional information below.

The problem is with strdup and free using 2 completely different methods for 
allocating and deallocating memory. The crash happens when we reach free.

bcrypt_python.c -> bcrypt_hashpw -> free(password_copy)

The problem is that cygwin / gcc links strdup with msvcrt.dll and free is 
linked with msvcr71.dll. These 2 are not supposed to work together. The fix is 
to either change the links in cygwin / gcc, or change the code.

I replaced the strdup with malloc and strcpy which seems to work fine.

Original issue reported on code.google.com by nagra...@cyaninc.com on 31 Oct 2011 at 11:10

GoogleCodeExporter commented 9 years ago
Can you be more specific on how you replaced 'strdup' and 'strcpy' with malloc 
please?

Original comment by rbfuente...@gmail.com on 14 May 2012 at 5:10

GoogleCodeExporter commented 9 years ago
To clear any possible misunderstandings, I replaced "strdup" with "malloc and 
strcpy". I did not replace "'strdup' and 'strcpy' with malloc". Here is the new 
method...

static PyObject *
bcrypt_hashpw(PyObject *self, PyObject *args, PyObject *kw_args)
{
    static char *keywords[] = { "password", "salt", NULL };
    char *password = NULL, *salt = NULL;
    char *ret;

    if (!PyArg_ParseTupleAndKeywords(args, kw_args, "ss:hashpw", keywords,
        &password, &salt))
                return NULL;

    char *password_copy = malloc (strlen (password) + 1);
    if (password_copy == NULL) return NULL;
    strcpy(password_copy, password);

    char *salt_copy = malloc (strlen (salt) + 1);
    if (salt_copy == NULL) return NULL;
    strcpy(salt_copy, salt);

    Py_BEGIN_ALLOW_THREADS;
    ret = pybc_bcrypt(password_copy, salt_copy);
    Py_END_ALLOW_THREADS;

    free(password_copy);
    free(salt_copy);
    if ((ret == NULL) ||
        strcmp(ret, ":") == 0) {
        PyErr_SetString(PyExc_ValueError, "Invalid salt");
        return NULL;
    }

    return PyString_FromString(ret);
}

Original comment by nag.ra...@gmail.com on 14 May 2012 at 6:10

GoogleCodeExporter commented 9 years ago
There is a simpler alternative to re-implementing strdup.

#if defined(_WIN32)
/* On Windows strdup is deprecated, replaced by the ISO C compliant _strdup. */
#define strdup _strdup
#endif

Original comment by pet...@gmail.com on 11 Jun 2012 at 1:13

GoogleCodeExporter commented 9 years ago
I've removed the strdup calls entirely in the current hg tip as part of 
python-3.x support (issue #5). This will be in the forthcoming py-bcrypt-0.4 
and is in the hg tree now.

Original comment by d...@djm.net.au on 27 Jul 2013 at 11:58