search

arlolra / otr

Off-the-Record Messaging Protocol implemented in JavaScript
https://arlolra.github.io/otr/
Mozilla Public License 2.0
458 stars 61 forks source link

HLP functions depend on internal representation of BigInts #39

Closed daira closed 11 years ago

daira commented 11 years ago

For example, HLP.twotothe at https://github.com/arlolra/otr/blob/master/lib/helpers.js#L185 depends on the representation in BigInt.js being a little-endian array of words (and on how signedness is represented). Is this intended, and is it a good idea?

(Note that the otr.js 2.6 implementation of HLP.twotothe did not depend on the representation. I haven't checked other HLP functions.)

arlolra commented 11 years ago

Hey @daira,

Let me preface by saying that I appreciate the effort you're making in auditing Cryptocat and I look forward to all the resulting improvements.

In general I agree that it's better not to be relying on implementation details of dependencies. Yes, this is a problem with several HLP functions and would probably be better served moving them to the BigInt.js fork I'm (unwittingly) maintaining. In the past, I've tried to have a few of these changes upstreamed, to no avail.

On that note, it may be worth exploring a rewrite with SJCL. For now though, I'll make the recommended changes.

arlolra commented 11 years ago

See: https://github.com/arlolra/otr/commit/ac9d74bb4c81555e817b67cbeb3cde4ccb79f229

arlolra commented 11 years ago

https://github.com/arlolra/otr/compare/separate

should address this issue

arlolra commented 11 years ago

Merged these changes in 3394d992fa369837cfd8313251e1c3313376f7ac. Please reopen if you feel this is still an issue.

daira commented 11 years ago

That looks much better, thanks.