ziglang/zig

secp256k1 scalar multiplication panics

guidovranken opened this issue · 3 comments

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.

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

@jedisct1

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.

1e0d492 should fix it.

Thanks!