Accessing Keyword Sets with wrong size causes an BufferUnderflow exception

[ogerboss]

When you try to access the keywords of a record where the KSIZ-argument is larger than the given array of keywords, this will trigger a Buffer-Underflow exception:

[EXCEPTION]  java.nio.BufferUnderflowException
        at java.nio.HeapByteBuffer.get(Unknown Source)
        at java.nio.ByteBuffer.get(Unknown Source)
        at lev.LShrinkArray.extract(LShrinkArray.java:163)Reqtificator.jar
        at skyproc.FormID.parseData(FormID.java:141)
        at skyproc.SubFormArray.parseData(SubFormArray.java:51)
        at skyproc.KeywordSet.parseData(KeywordSet.java:68)
        at skyproc.SubRecordsStream.loadFromPosition(SubRecordsStream.java:148)
        at skyproc.SubRecordsStream.get(SubRecordsStream.java:53)
        at skyproc.SubRecords.getKeywords(SubRecords.java:151)
        at skyproc.ARMO.getKeywordSet(ARMO.java:130) 
        at RequiemForTheIndifferent.components.Equipment.reqtify_armor(Equipment.java:84)
        at RequiemForTheIndifferent.RequiemForTheIndifferent.runChangesToPatch(RequiemForTheIndifferent.java:286)
        at skyproc.gui.SUMGUI$ProcessingThread.run(SUMGUI.java:952)
        at java.lang.Thread.run(Unknown Source)

Please see my Requiem blogpost for a detailed analysis of the issue.

[ogerboss]

I tried out the corresponding dev-branch from your forked repository an quick test yielded the attached results.

The Skyproc-processed record now has the correct keyword-count, the adding of the duplicate ArmorHeavy keyword no longer causes an exception and the other changes made by the patch are also applied as expected.

[ogerboss]

please keep this issue open for now, I will create a flawed plugin for testing tomorrow that has less keywords than expected, to test if the opposite case also works fine now.

[ogerboss]

I just tested this with a 9 keyword item that I gave a KSIZ of 10. The last release version caused a BufferUnderflow again, while the updated version corrected the value to 9 without raising any exceptions. :) Seems to be fixed.

Disclaimer: I used the 73dd23a commit which no longer exists, but a diff showed it's identical to the one now found in the master-branch of the dev-fork. :)

[David Tynan]

Great. It'll be in 2.2.0.7

And yes I rebase commits on my dev branch because otherwise it ends up a huge mess.

[ogerboss]

small appendix: It just occured to me that I only tested armors so far... Thus I also tested wrong KSIZ-arguments for weapons, with the same results :)

Any ETA for 2.2.0.7? I need to release a patcher update for this issue and the language issues soon. Thus I would like to know if it's worth to wait for the next release or if I need to use a dev-build.

[David Tynan]

The fix was in the class that handles all subrecords with a size value and then entries so it ought to work for all keywords and maybe some other things.

I was shooting for this weekend for 2.2.0.7. Qotsafan is done with PERKs and I'm close with QUST so I need to finish that and review his work. He's also taking a whack at some other stuff like the language from ini and better missingMaster exceptions. That said I'm pretty sure the master line of my repo should be safe to use if you want to get a new build out ASAP.