matter-labs / bellman

Bellman zkSNARK library for community with Ethereum's BN256 support
https://matter-labs.io
Other
160 stars 79 forks source link

Using pairing_ce verison 0.21 will cause pinic when handle GroupDecodingError? #23

Closed XuyangSong closed 4 years ago

XuyangSong commented 4 years ago

https://github.com/matter-labs/bellman/blob/8a96585679a8bee091d510a931e9c2d79b7a3e0b/src/groth16/mod.rs#L65 I choose a false case using an invalid proof(a point is not on curve). It will be panic here.

When input an invalid ec point and call into_affine, then get GroupDecodingError. It will cause panic, stack overflowed.

The error may be from the recent commit in pairing_ce verison 0.21. https://github.com/matter-labs/pairing/blob/977b192b3fe9a14acf943fa0f04a41add1208725/src/lib.rs#L330 In pairing_ce verison 0.21, modified to "self.to_string()" from "self.description()" would cause recursive calling.

shamatar commented 4 years ago

Hey @TheWhiteShell

It’s a nice catch by I do not yet understand why it could be a reason. description() is deprecated so I’ve updated to to_string(), it should not include any recursive calls. Can you poke it a little more (like deserialize a single invalid point from buffer) and see what happens?

XuyangSong commented 4 years ago

I construct a simple test case in pairing_ce verison 0.21.

#[test] fn test_groupdecodingerror_display() { let err = GroupDecodingError::NotOnCurve; println!("{}", err); }

It will get: thread 'test_groupdecodingerror_display' has overflowed its stack fatal runtime error: stack overflow

"err" will display and then call " _ => write!(f, "{}", self.to_string()),"

"self.tostring()" display and then call " => write!(f, "{}", self.to_string())," again

It will be recursive.

shamatar commented 4 years ago

Ok, I’ll fix it later this week

shamatar commented 4 years ago

pairing_ce was updated to fix this, should be enough to run cargo update

XuyangSong commented 4 years ago

@shamatar

https://github.com/matter-labs/bellman/blob/8a96585679a8bee091d510a931e9c2d79b7a3e0b/src/cs.rs#L202

Another panic, the same error.