Apply fix of BCEL-363 (CVE-2022-42920) to AspectJ's "own BCEL"

[daniel-matheis-vivavis]

see BCEL issue https://issues.apache.org/jira/browse/BCEL-363
resp. apache/commons-bcel#147

It seems that Aspectj's "own BCEL" also contains this problem.

[kriegaex]

Very well possible, AspectJ's BCEL is derived from an ancient version. Which concrete problem are you running into with AspectJ? Where is a reproducer for that problem?

[daniel-matheis-vivavis]

The regular user of AspectJ won't have a problem with that security issue I would say, as long as AspectJ does not try to write "too many" constant pool entries which would corrput the class file.
It is just that the contained code is (nearly) identical to the one that was fixed elsewhere and thus contains the problem described by the CVE-2022-42920 (which does not tell how to exploit that actually.)

Generally would be interesting to know why this "AspectJ private BCEL" is/was necessary.
Wouldn't it be best not to repackage it but to use the "latest and greatest" BCEL as external dependency?

[kriegaex]

Generally would be interesting to know why this "AspectJ private BCEL" is/was necessary.

That was done many years before I started contributing to AspectJ. I think BCEL was used as a starting point for AspectJ-specific modifications and maybe refreshed from upstream in the olden days, but then it ended up simply being updated manually as the Java language and AspectJ evolved. Now the code bases have diverged so much that it would be a major effort (maybe at the scale of a rewrite) to separate our changes from the original BCEL and keep it in a separate, updateable patch state. BTW, AspectJ also uses ASM, and if AspectJ was to be rewritten today, probably it should use only of of them.

Wouldn't it be best not to repackage it but to use the "latest and greatest" BCEL as external dependency?

Like I said before, it is not just an inlined BCEL version but a heavily modified one.

[aclement]

Generally would be interesting to know why this "AspectJ private BCEL" is/was necessary

Basically we needed to move to a lazier model of unpacking class files for performance/resource-usage reasons (for example, don't unnecessarily unpack the line number table if no-one is asking for line number data or making changes that cause it to need adjustment). If revisiting it now, we'd probably switch to Asm because we also have to use asm to compute stack maps. (unless bcel has added that? maybe it has by now? I seem to recall at the time, many years ago, bcel was in no longer really being maintained).