search

kazu-yamamoto / pgpdump

A PGP packet visualizer
http://www.mew.org/~kazu/proj/pgpdump/
BSD 3-Clause "New" or "Revised" License
174 stars 34 forks source link

Update keys.c #34

Closed galaxiesFarApart closed 2 years ago

galaxiesFarApart commented 2 years ago

2022-03-01: insert "memset" where ELLIP_CURVES are evaluated to re-initialize the "oid_input_HEX" array to all zero; left as it was, multiple different ECC curve evaluations may leave remnants of earlier read input values which can cause "memcpy" to fail (for "invalid/undefined/unmatched values") -> segfault; most notable when evaluating "keys" with a plethora of packets with different ECC algorithms

galaxiesFarApart commented 2 years ago

Ah - phooey. I didn't really mean to close PR#34 (having problems determining status of created PRs)

galaxiesFarApart commented 2 years ago

Attached are dump outputs and test key file that results in segfault (which really occurs in "printf" statement). Same files have also been sent through e-mail.

ggp234-ecc-Brainpool-P256-test-2022-03-01.pubkey.zip Multiple_ECC_algos_with_many_packets-segfault-2022-03-01.log.zip Multiple_ECC_algos_with_many_packets-with_memset-2022-03-01.log.zip

kazu-yamamoto commented 2 years ago

Cherry-picked and merged. Thank you for your contribution!

galaxiesFarApart commented 2 years ago

Yes - using "size" can be done. I had thought of not using 10 realizing that that value could change in the future, but I already had that value established from RFCs. I will work on the change.

Sent from my iPad - J.Dutton

On Apr 12, 2022, at 11:31 PM, Kazu Yamamoto @.***> wrote:

 @kazu-yamamoto requested changes on this pull request.

Sorry for the delay. And nice catch!

In keys.c:

@@ -125,6 +125,7 @@ new_Public_Key_Packet(int len) break; case 18:/ECDH/ oidLEN = Getc();

  • memset(oid_input_HEX,0,10); I would like to avoid hard-coding a magic number. Could you use oid_input_HEX_size instead of 10?

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you modified the open/close state.