Skip to content
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

Is FieldVal.AddInt() in btcec really safe for overflow? #1171

Open
chaintechlab opened this issue Apr 25, 2018 · 1 comment
Open

Is FieldVal.AddInt() in btcec really safe for overflow? #1171

chaintechlab opened this issue Apr 25, 2018 · 1 comment

Comments

@chaintechlab
Copy link

func (f *fieldVal) AddInt(ui uint) *fieldVal {
    // Since the field representation intentionally provides overflow bits,
    // it's ok to use carryless addition as the carry bit is safely part of
    // the word and will be normalized out.
    f.n[0] += uint32(ui)
    return f
}

I think it is not really safe here. If ui is uint32::MAX and f.n[0] is 1, what will happen?
BTW, it is not proper here to cast uint to uint32 (on 64bit syste, uint is uint64).

@stevenroose
Copy link
Contributor

It is documented that that function is intended for usage with small numbers:

// AddInt adds the passed integer to the existing field value and stores the
// result in f.  This is a convenience function since it is fairly common to
// perform some arithemetic with small native integers.
//
// The field value is returned to support chaining.  This enables syntax like:
// f.AddInt(1).Add(f2) so that f = f + 1 + f2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants