When multiple threads perform encryption/decryption on the same Blowfish object (as is the case with the Blowfish object supplied by the AuthFactory), the results may come out wrong.
The issue rises from the fact that the Blowfish encryption and decryption aren’'t atomic operations and thus threads may interfere with each other, causing unexpected results.
The problem with this implementation is that access to the m_bfish member isn’'t synchronized so that threads may alter its internal state while encryption/decryption is being performed on another thread.
This code in the Blowfish’'s encStr method demonstrates the issue:
.
.
.
// create the encryptor
m_bfish.setCBCIV(lNewCBCIV);
// Another thread may execute here, altering the CBCIV and affecting the result of this
// thread’'s encryption operation. // encrypt the buffer
m_bfish.encrypt(buf);
.
.
.
This issue also applies to the decryptString method.
One solution to this problem could be to synchronize the access to the m_bfish object during encryption/decryption:
synchronized (m_bfish)
{
// create the encryptor
m_bfish.setCBCIV(lNewCBCIV);
// encrypt the buffer
m_bfish.encrypt(buf);
}
as well as declaring all methods of the BlowfishCBC class that alter the CBCIV (the m_lCBCIV member) synchronized.
You could also synchronize on a higher level, like the Blowfish’‘s encryptString/decryptString methods, or even in the AuthFactory’'s
encryptPassword/decryptPassword methods.