Opened 11 years ago

Closed 6 years ago

#7264 closed defect (wontfix)

Blowfish: sign extension bit issue (UTF encoding)

Reported by: rdunklau Owned by: Tom Trenka
Priority: low Milestone: 2.0
Component: Dojox Version: 1.1.1
Keywords: Blowfish Crypto Encoding dojox.encoding.crypto reviewed Cc:
Blocked By: Blocking:

Description

There is a bug regarding the sign extension bit in the blowfish implementation. The current implementation doesn't take care of this case.

You can reproduce it with the attached "showBug.js" file.

The blowFish.js file contains the correction.

Basically , you have to take the charcode % 256 before encrypting :

for(var i=0; i<count; i++){
			o.left=(plaintext.charCodeAt(pos) % 256)*POW24 
				|(plaintext.charCodeAt(pos+1) % 256)*POW16
				|(plaintext.charCodeAt(pos+2) % 256)*POW8
				|(plaintext.charCodeAt(pos+3) % 256);
			o.right=(plaintext.charCodeAt(pos+4) % 256)*POW24
				|(plaintext.charCodeAt(pos+5) % 256)*POW16
				|(plaintext.charCodeAt(pos+6) % 256)*POW8
				|(plaintext.charCodeAt(pos+7) % 256);

And correct the charcode after decrypting :

//convert to string
return dojo.map(pt, function(item){
   if(item > 127){
      item = 65536 - 256 + item;		
   }
   return String.fromCharCode(item);
}).join("");	//	string

Attachments (2)

Blowfish.js (22.9 KB) - added by rdunklau 11 years ago.
"Patched" BlowFish?.js file
testCase.js (469 bytes) - added by rdunklau 11 years ago.
A simple test case

Download all attachments as: .zip

Change History (16)

Changed 11 years ago by rdunklau

Attachment: Blowfish.js added

"Patched" BlowFish?.js file

comment:1 Changed 11 years ago by Adam Peller

Owner: changed from Adam Peller to Tom Trenka

Changed 11 years ago by rdunklau

Attachment: testCase.js added

A simple test case

comment:2 Changed 11 years ago by Tom Trenka

Milestone: tbd1.3

comment:3 Changed 11 years ago by Tom Trenka

severity: majornormal

comment:4 Changed 11 years ago by Tom Trenka

Does this change/patch pass the test vectors that are public domain? See the Bouncy Castle code base...

comment:5 Changed 11 years ago by rdunklau

Sorry, but i did not found the test vectors. Could you tell me a bit more ? Thanks ! (And sorry again for being so late... )

comment:6 Changed 11 years ago by Tom Trenka

Milestone: 1.31.4
Status: newassigned

Realizing that this fix assumes non-ASCII characters, and the original implementation (as with all of the other crypto/digest implementations) are only supporting ASCII at the moment.

Going to defer this to at least 1.4, or when we add full unicode support to all of the crypto/digest algorithms, for consistency.

comment:7 Changed 10 years ago by Tom Trenka

Milestone: 1.41.5

Deferring to 1.5; have not had the chance to go through and implement UTF encoding for any of the digests or crypto algorithms yet.

comment:8 Changed 10 years ago by Tom Trenka

Milestone: 1.51.6

...and again deferring this to 1.6, because of the accelerated release schedule for 1.5.

comment:9 Changed 9 years ago by Tom Trenka

Summary: Bug in BlowfishBlowfish: sign extension bit issue (UTF encoding)

comment:10 Changed 9 years ago by bill

Milestone: 1.6future

(sadly) punting seemingly abandoned ticket and meta tickets to future

comment:11 Changed 8 years ago by Tom Trenka

Milestone: future1.8
Priority: highlow

comment:12 Changed 8 years ago by Tom Trenka

Keywords: reviewed added

comment:13 Changed 7 years ago by Colin Snover

Milestone: 1.82.0

1.8 has been tagged; moving all outstanding tickets to next major release milestone.

comment:14 Changed 6 years ago by Tom Trenka

Resolution: wontfix
Status: assignedclosed

Given the age of these tickets and the small nature of them, I'm marking all as won't fix. #1395 was somewhat addressed with the implementation of SHA2 in DTK 1.8.

Note: See TracTickets for help on using tickets.