4 ms·

Also note the same user owns this library: https://github.com/right9ctrl/node-scrypt https://github.com/right9ctrl/node-scrypt I would be very suspicious of t

by rdtsc 8y ago

Also note the same user owns this library: https://github.com/right9ctrl/node-scrypt https://github.com/right9ctrl/node-scrypt I would be very suspicious of that as well and audit anything that library has touched.

luizfzs 8y ago
A funny thing is that on README.md, the `Install from source` section has this: `git clone https://github.com/barrysteyn/node-scrypt.git` https://github.com/barrysteyn/node-scrypt.git`.
eropple 8y ago
Good eyes, but this looks like it may be a red herring. That is a fork of the `scrypt` package, canonically found here: https://www.npmjs.com/package/scrypt https://www.npmjs.com/package/scrypt With one maintainer, Barry Steyn, whose referenced repo is here: https://github.com/barrysteyn/node-scrypt https://github.com/barrysteyn/node-scrypt Somebody would have to pick it up from a git URL, I think. (Maybe they could typosquat, but we'd have to find that first.)
- rdtsc 8y ago
  Yeah it looks mostly like a copy and saw most commits are not by that user, but the user has made some commits and not being familiar with how NPM packages work too well, thought I'd drop a warning here.
luizfzs 8y ago
I'm not a crypto guy but this change really makes me scratch my head: https://github.com/right9ctrl/node-scrypt/commit/52a1cb792bc7ae0e4051ba4ad53e7b60fa5c806d#diff-91bff4c651c287ea055148b8b3eb1062 https://github.com/right9ctrl/node-scrypt/commit/52a1cb792bc...
- OJFord 8y ago
  Yes, that looks dodgy - a 64-bit 'N' being renamed 'logN', and its value changed to `1` left shifted by its prior self? I think there's a good bet that's just zeroing it out.
  - XCSme 8y ago
    It's just doing 2^logN...
    - deleted 8y ago
      [deleted]
    - OJFord 8y ago
      Which ought to be just N - but notice the argument passed in is mysteriously renamed. It may be nothing - I'm not even sure what N is here - I just agree that it looks weird.
      - jnewell 8y ago
        https://github.com/barrysteyn/node-scrypt/pull/119 https://github.com/barrysteyn/node-scrypt/pull/119
- regularfry 8y ago
  That just looks like a convenience function to me. Rather than passing 1,2,4,8,16 in as the value of N, you pass in 0,1,2,3,4. Has the benefit that it's not possible to pass in a number that's not a power of 2 (which might not be valid, I don't know how scrypt works).
  - CiPHPerCoder 8y ago
    IIRC scrypt is only defined for powers of 2 for N.

by rdtsc 8y ago

luizfzs 8y ago

A funny thing is that on README.md, the `Install from source` section has this: `git clone https://github.com/barrysteyn/node-scrypt.git` https://github.com/barrysteyn/node-scrypt.git`.

eropple 8y ago

Good eyes, but this looks like it may be a red herring. That is a fork of the `scrypt` package, canonically found here: https://www.npmjs.com/package/scrypt https://www.npmjs.com/package/scrypt With one maintainer, Barry Steyn, whose referenced repo is here: https://github.com/barrysteyn/node-scrypt https://github.com/barrysteyn/node-scrypt Somebody would have to pick it up from a git URL, I think. (Maybe they could typosquat, but we'd have to find that first.)

rdtsc 8y ago
Yeah it looks mostly like a copy and saw most commits are not by that user, but the user has made some commits and not being familiar with how NPM packages work too well, thought I'd drop a warning here.

luizfzs 8y ago

I'm not a crypto guy but this change really makes me scratch my head: https://github.com/right9ctrl/node-scrypt/commit/52a1cb792bc7ae0e4051ba4ad53e7b60fa5c806d#diff-91bff4c651c287ea055148b8b3eb1062 https://github.com/right9ctrl/node-scrypt/commit/52a1cb792bc...

OJFord 8y ago
Yes, that looks dodgy - a 64-bit 'N' being renamed 'logN', and its value changed to `1` left shifted by its prior self? I think there's a good bet that's just zeroing it out.
- XCSme 8y ago
  It's just doing 2^logN...
  - deleted 8y ago
    [deleted]
  - OJFord 8y ago
    Which ought to be just N - but notice the argument passed in is mysteriously renamed. It may be nothing - I'm not even sure what N is here - I just agree that it looks weird.
    - jnewell 8y ago
      https://github.com/barrysteyn/node-scrypt/pull/119 https://github.com/barrysteyn/node-scrypt/pull/119
regularfry 8y ago
That just looks like a convenience function to me. Rather than passing 1,2,4,8,16 in as the value of N, you pass in 0,1,2,3,4. Has the benefit that it's not possible to pass in a number that's not a power of 2 (which might not be valid, I don't know how scrypt works).
- CiPHPerCoder 8y ago
  IIRC scrypt is only defined for powers of 2 for N.