staktrace / cafebabe

A java class file parser
43 stars 12 forks source link

Fix: Prevent out of range reads in ConstantPoolIter #31

Closed pradt2 closed 1 year ago

pradt2 commented 1 year ago

Hi,

It is currently possible (and it fact, it has happened to me on a few occasions) that the ConstantPoolIter panicks with an out-of-range read.

This PR fixes it.

staktrace commented 1 year ago

Can you add a test that demonstrates the problem?

pradt2 commented 1 year ago

@staktrace Done. If you paste the added test in the current master, you'll see that it panics.

staktrace commented 1 year ago

Sorry, I should have been more clear - do you have a real world example that this fails on? Like a .class file that the JVM considers valid, rather than just constructing an invalid constant pool in code.

pradt2 commented 1 year ago

Dude wtf... Just look at your code, if the last attribute is a UTF8 string it will clearly fail.

Or, you know, maybe take 1 min to try your code on a HelloWorld example before releasing it to the public (remove .txt from filename first)...

HelloWorld.class.txt HelloWorld.java.txt

staktrace commented 1 year ago
  1. I know this will come as an absolute shock to you, but my life doesn't revolve around this crate. There's a reason I marked it passively-maintained. I don't even use this crate any more myself since I wrote it as a one-off for something which is now done.
  2. If you plan to continue using and working with open source, read up on open source etiquette and check your entitled attitude at the door. I've pushed a fix for this bug and released it in v0.6.1 because I take pride in my work and this was a stupid mistake, not because of anything you've said or done so far.
  3. On a technical level, the error occurs on any constant pool, not just ones that have a UTF8 string as the last item. It's an off-by-one error and is a trivial fix. I don't know why your PR rewrites the entire flow of the function to be more complex and inefficient.