New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proper address checking for all CryptoNote currencies #48

Open
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@fancoder

fancoder commented Jul 3, 2014

Added new setting config.poolServer.addressBase58Prefix. Should be
the same as CRYPTONOTE_PUBLIC_ADDRESS_BASE58_PREFIX in
src/cryptonote_config.h currency config.

fancoder added some commits Jul 3, 2014

Proper address checking for all CryptoNote currencies
Added new setting config.poolServer.addressBase58Prefix. Should be
the same as CRYPTONOTE_PUBLIC_ADDRESS_BASE58_PREFIX in
src/cryptonote_config.h currency config.
@mapleshadow

This comment has been minimized.

mapleshadow commented Jul 11, 2014

qq20140711173802
Does not work, invalid! Testing is AEON

@fancoder

This comment has been minimized.

fancoder commented Jul 15, 2014

It was fixed on my branch a week ago. Forgot to update this push request. Now I've done it.
Thanks for your feedback.

@mapleshadow

This comment has been minimized.

mapleshadow commented Jul 16, 2014

Advised to turn off, the current version, this problem does not exist

@fancoder

This comment has been minimized.

fancoder commented Jul 16, 2014

The problem with address validation does exist. It is a significant vulnerability of node-cryptonote-pool. Could you give me a link to your pool, so I could show you that there is a problem with it? :)

As for the bug you've mentioned, it was fixed with my latest pull request: fancoder@7b46a02

@dayas

This comment has been minimized.

dayas commented Jul 16, 2014

address validation works perfectly. This code is specific for Monero, and has no issues with address validation.

If you are running this code for other coins, such as Ducknote, then yes there is issues but again this code is not written for ducknote it is written for Monero.

@fancoder

This comment has been minimized.

fancoder commented Jul 16, 2014

dayas, you're getting it wholly wrong. There is no difference in address validation algorithm for CryptoNote coins. The one used originally in node-cryptonote-pull is subject to a flaw which may result in ANY cryptonote pool being attacked and brought down. Just give me a link to your pool, I can show you.

CryptoNote's addresses are not defined by the first letter and the length only, but also through more rules. The address consists of:

  • address prefix (determines the currency)
  • spending public key
  • tracking public key

The original method doesn't validate the key pairs, which may lead to the improper address being accepted by the pool, but breaking the payout and freezing the pool.

@mapleshadow

This comment has been minimized.

mapleshadow commented Jul 16, 2014

Well, yes, indeed it is.
But I suggest, please use the latest code fixes, this should be officially
accepted

2014-07-16 23:45 GMT+08:00 CliffordST notifications@github.com:

dayas, you're getting it wholly wrong. There is no difference in address
validation algorithm for CryptoNote coins. The one used originally in
node-cryptonote-pull is subject to a flaw which may result in ANY
cryptonote pool being attacked and brought down. Just give me a link to
your pool, I can show you.

CryptoNote's addresses are not defined by the first letter and the length
only, but also through more rules. The address consists of:

  • address prefix (determines the currency)
  • spending public key
  • tracking public key

The original method doesn't validate the key pairs, which may lead to the
improper address being accepted by the pool, but breaking the payout and
freezing the pool.


Reply to this email directly or view it on GitHub
#48 (comment)
.

—枫影—

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment