Hi Eliot,
On Thu, 12 Apr 2018, Eliot Miranda wrote:
[I'm sorry, I apparently forgot to hit send some weeks ago]
Well, I had started to rewrite the methods myself along with adding tests, but you were quicker and pushed the changes.
I think there are still things that should be fixed. When time allows, I'll check my tests and merge them with the existing MiscPrimitivePluginTest.
stores unsigned ints (data) into itprimitiveDecompressFromByteArray
- bm is int*, but it should probably be unsigned int* as the method
- index should be bounds checked (1 .. end)
It gets checked anyway with the comparison against pastEnd
Unfortunately that's not the case. i is compared to end, not pastEnd.
If index, which is an unnecessary variable btw, is less than or equal to 0, then the value of i will be negative, and the input will be processed no matter what the value of end is, hence random data from memory will be read and processed:
Smalltalk garbageCollect.
ba := ByteArray new: 256.
bm := Bitmap new: 1024.
passes := OrderedCollection new.
success := true.
[
0 to: -1000000 by: -1 do: [ :start |
success := true.
bm decompress: bm fromByteArray: ba at: start.
success ifTrue: [ passes add: start ] ] ]
on: Error
do: [ :error |
success := false.
error return: nil ].
passes
- i, k, end and pastEnd should be unsigned int
That's OK. In neither V3 nor Spur can objects have more than 1Gb fields so in practice indices can never go negative.
If i's type were unsigned int, the above code wouldn't work, because i < end would be false.
Levente