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

secp256k1 scalar multiplication panics #15267

Closed
guidovranken opened this issue Apr 13, 2023 · 3 comments · Fixed by #15270
Closed

secp256k1 scalar multiplication panics #15267

guidovranken opened this issue Apr 13, 2023 · 3 comments · Fixed by #15270
Labels
bug Observed behavior contradicts documented or intended behavior standard library This issue involves writing Zig code for the standard library.
Milestone

Comments

@guidovranken
Copy link

Zig Version

zig-linux-x86_64-0.11.0-dev.2560+602029bb2

Steps to Reproduce and Observed Behavior

const std = @import("std");
const Secp256k1 = std.crypto.ecc.Secp256k1;

pub fn main() !void {
    var x: [32]u8 = [32]u8{
        0x79, 0xBE, 0x66, 0x7E, 0xF9, 0xDC, 0xBB, 0xAC, 0x55, 0xA0, 0x62, 0x95, 0xCE, 0x87, 0x0B, 0x07,
        0x02, 0x9B, 0xFC, 0xDB, 0x2D, 0xCE, 0x28, 0xD9, 0x59, 0xF2, 0x81, 0x5B, 0x16, 0xF8, 0x17, 0x98,
    };
    var y: [32]u8 = [32]u8{
        0xB7, 0xC5, 0x25, 0x88, 0xD9, 0x5C, 0x3B, 0x9A, 0xA2, 0x5B, 0x04, 0x03, 0xF1, 0xEE, 0xF7, 0x57,
        0x02, 0xE8, 0x4B, 0xB7, 0x59, 0x7A, 0xAB, 0xE6, 0x63, 0xB8, 0x2F, 0x6F, 0x04, 0xEF, 0x27, 0x77,
    };
    var b: [32]u8 = [32]u8{
        0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
        0xFF, 0xFF, 0xFF, 0xB0, 0x7F, 0x6D, 0x60, 0x8E, 0x4D, 0xCC, 0x38, 0x02, 0x0B, 0x14, 0x0B, 0x36,
    };
    var a = Secp256k1.fromSerializedAffineCoordinates(x, y, .Big) catch return;
    _ = a.mulPublic(b[0..32].*, .Big) catch return;
}
thread 1365119 panic: attempt to unwrap error: NonCanonical
/snap/zig/6352/lib/std/crypto/pcurves/common.zig:61:17: 0x27c756 in rejectNonCanonical (p)
                return error.NonCanonical;
                ^
/snap/zig/6352/lib/std/crypto/pcurves/common.zig:75:13: 0x2730d5 in fromBytes (p)
            try rejectNonCanonical(s, .Little);
            ^
/snap/zig/6352/lib/std/crypto/pcurves/secp256k1/scalar.zig:87:30: 0x24cd0a in fromBytes (p)
        return Scalar{ .fe = try Fe.fromBytes(s, endian) };
                             ^
/snap/zig/6352/lib/std/crypto/pcurves/secp256k1/scalar.zig:67:13: 0x24cb25 in sub (p)
    return (try Scalar.fromBytes(a, endian)).sub(try Scalar.fromBytes(b, endian)).toBytes(endian);
            ^
/snap/zig/6352/lib/std/crypto/pcurves/secp256k1.zig:84:51: 0x2176de in splitScalar (p)
            r1 = scalar.sub(s, r1, .Little) catch unreachable;
                                                  ^
/snap/zig/6352/lib/std/crypto/pcurves/secp256k1.zig:446:53: 0x213ae0 in mulPublic (p)
        var split_scalar = Endormorphism.splitScalar(s, .Little);
                                                    ^
./p.zig:18:22: 0x21365b in main (p)
    _ = a.mulPublic(b[0..32].*, .Big) catch return;
                     ^
/snap/zig/6352/lib/std/start.zig:614:37: 0x21303d in posixCallMainAndExit (p)
            const result = root.main() catch |err| {
                                    ^
/snap/zig/6352/lib/std/start.zig:376:5: 0x212b21 in _start (p)
    @call(.{ .modifier = .never_inline }, posixCallMainAndExit, .{});
    ^
Aborted (core dumped)

Expected Behavior

No panic, but an exception which is caught by the invocation to mulPublic.

@guidovranken guidovranken added the bug Observed behavior contradicts documented or intended behavior label Apr 13, 2023
@guidovranken
Copy link
Author

Not sure about this but it seems an exception is more appropriate?

@jedisct1

@jedisct1
Copy link
Contributor

Thank you Guido!

Yes, we check everywhere that inputs are in canonical form, so returning an error is the right thing to do, and doesn't cost extra CPU cycles.

jedisct1 added a commit to jedisct1/zig that referenced this issue Apr 13, 2023
@jedisct1
Copy link
Contributor

1e0d492 should fix it.

Thanks!

@andrewrk andrewrk added the standard library This issue involves writing Zig code for the standard library. label Apr 13, 2023
@andrewrk andrewrk added this to the 0.11.0 milestone Apr 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Observed behavior contradicts documented or intended behavior standard library This issue involves writing Zig code for the standard library.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants