From 34fa05da9715a3f5a3359d37bffc2ef4c2a50a2f Mon Sep 17 00:00:00 2001 From: Ryan Oldenburg Date: Mon, 16 May 2022 18:08:18 -0500 Subject: [PATCH 1/7] simpler --- src/pixie/fileformats/jpeg.nim | 47 +++++++++++----------------------- 1 file changed, 15 insertions(+), 32 deletions(-) diff --git a/src/pixie/fileformats/jpeg.nim b/src/pixie/fileformats/jpeg.nim index 2fb59e2..9537aab 100644 --- a/src/pixie/fileformats/jpeg.nim +++ b/src/pixie/fileformats/jpeg.nim @@ -474,9 +474,8 @@ proc decodeRegularBlock( ) = ## Decodes a whole block. let t = state.huffmanDecode(0, state.components[component].huffmanDC).int - if t < 0: - failInvalid() - + if t > 15: + failInvalid("bad huffman code") let diff = if t == 0: @@ -488,7 +487,7 @@ proc decodeRegularBlock( data[0] = cast[int16](dc) var i = 1 - while true: + while i < 64: let rs = state.huffmanDecode(1, state.components[component].huffmanAC) s = rs and 15 @@ -499,15 +498,12 @@ proc decodeRegularBlock( i += 16 else: i += r.int - if i notin 0 ..< 64: + if i >= 64: failInvalid() let zig = deZigZag[i] data[zig] = cast[int16](state.getBitsAsSignedInt(s.int)) inc i - if not(i < 64): - break - proc decodeProgressiveBlock( state: var DecoderState, component: int, data: var array[64, int16] ) = @@ -517,18 +513,17 @@ proc decodeProgressiveBlock( if state.successiveApproxHigh == 0: let t = state.huffmanDecode(0, state.components[component].huffmanDC).int - if t < 0 or t > 15: - failInvalid() - let - diff = if t != 0: - state.getBitsAsSignedInt(t) - else: - 0 + if t > 15: + failInvalid("bad huffman code") let + diff = + if t > 0: + state.getBitsAsSignedInt(t) + else: + 0 dc = state.components[component].dcPred + diff state.components[component].dcPred = dc data[0] = cast[int16](dc * (1 shl state.successiveApproxLow)) - else: if getBit(state) != 0: data[0] = cast[int16](data[0] + (1 shl state.successiveApproxLow)) @@ -548,12 +543,9 @@ proc decodeProgressiveContinuationBlock( return var k = state.spectralStart - while true: + while k <= state.spectralEnd: let rs = state.huffmanDecode(1, state.components[component].huffmanAC) - if rs < 0: - failInvalid("bad huffman code") - let s = rs and 15 r = rs.int shr 4 if s == 0: @@ -566,7 +558,7 @@ proc decodeProgressiveContinuationBlock( k += 16 else: k += r.int - if k notin 0 ..< 64: + if k >= 64: failInvalid() let zig = deZigZag[k] inc k @@ -574,9 +566,6 @@ proc decodeProgressiveContinuationBlock( failInvalid() data[zig] = cast[int16](state.getBitsAsSignedInt(s.int) * (1 shl shift)) - if not(k <= state.spectralEnd): - break - else: var bit = 1 shl state.successiveApproxLow @@ -593,11 +582,8 @@ proc decodeProgressiveContinuationBlock( data[zig] = cast[int16](data[zig] - bit) else: var k = state.spectralStart - while true: - let - rs = state.huffmanDecode(1, state.components[component].huffmanAC) - if rs < 0: - failInvalid("bad huffman code") + while k <= state.spectralEnd: + let rs = state.huffmanDecode(1, state.components[component].huffmanAC) var s = rs.int and 15 r = rs.int shr 4 @@ -633,9 +619,6 @@ proc decodeProgressiveContinuationBlock( break dec r - if not (k <= state.spectralEnd): - break - template idct1D(s0, s1, s2, s3, s4, s5, s6, s7: int32) = ## Inverse discrete cosine transform 1D template f2f(x: float32): int32 = (x * 4096 + 0.5).int32 From 50090a88005154d0341e4550aa1e90b359ad5e89 Mon Sep 17 00:00:00 2001 From: Ryan Oldenburg Date: Mon, 16 May 2022 18:18:30 -0500 Subject: [PATCH 2/7] f --- src/pixie/fileformats/jpeg.nim | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/pixie/fileformats/jpeg.nim b/src/pixie/fileformats/jpeg.nim index 9537aab..dcc9d92 100644 --- a/src/pixie/fileformats/jpeg.nim +++ b/src/pixie/fileformats/jpeg.nim @@ -757,7 +757,7 @@ proc checkReset(state: var DecoderState) = state.fillBitBuffer() if state.buffer[state.pos] == 0xFF.char: - if state.buffer[state.pos+1] in {0xD0.char .. 0xD7.char}: + if state.buffer[state.pos + 1] in {0xD0.char .. 0xD7.char}: state.pos += 2 else: failInvalid("did not get expected reset marker") @@ -892,15 +892,12 @@ proc buildImage(state: var DecoderState): Image = cb.unsafe[x, y], cr.unsafe[x, y], ) - elif state.components.len == 1: let cy = state.components[0].channel for y in 0 ..< state.imageHeight: for x in 0 ..< state.imageWidth: - result.unsafe[x, y] = grayScaleToRgbx( - cy.unsafe[x, y], - ) + result.unsafe[x, y] = grayScaleToRgbx(cy.unsafe[x, y]) else: failInvalid() From 4984513e227a06d898536b27d051bb75c13cd829 Mon Sep 17 00:00:00 2001 From: Ryan Oldenburg Date: Mon, 16 May 2022 18:25:53 -0500 Subject: [PATCH 3/7] f --- src/pixie/fileformats/jpeg.nim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pixie/fileformats/jpeg.nim b/src/pixie/fileformats/jpeg.nim index dcc9d92..68dcd99 100644 --- a/src/pixie/fileformats/jpeg.nim +++ b/src/pixie/fileformats/jpeg.nim @@ -311,7 +311,7 @@ proc reset(state: var DecoderState) = if state.restartInterval != 0: state.todo = state.restartInterval else: - state.todo = 0x7fffffff + state.todo = int.high state.eobRun = 0 proc decodeSOS(state: var DecoderState) = From 30fc7c73c9f2d5f161a6c36470b65db5b77ab549 Mon Sep 17 00:00:00 2001 From: Ryan Oldenburg Date: Mon, 16 May 2022 18:32:53 -0500 Subject: [PATCH 4/7] reset marker -> restart marker --- src/pixie/fileformats/jpeg.nim | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/pixie/fileformats/jpeg.nim b/src/pixie/fileformats/jpeg.nim index 68dcd99..54abc49 100644 --- a/src/pixie/fileformats/jpeg.nim +++ b/src/pixie/fileformats/jpeg.nim @@ -79,7 +79,7 @@ type componentOrder: seq[int] progressive: bool restartInterval: int - todo: int + todoBeforeRestart: int eobRun: int when defined(release): @@ -117,7 +117,7 @@ proc skipChunk(state: var DecoderState) = proc decodeDRI(state: var DecoderState) = ## Decode Define Restart Interval - var len = state.readUint16be() - 2 + let len = state.readUint16be() - 2 if len != 2: failInvalid("invalid DRI length") state.restartInterval = state.readUint16be().int @@ -302,16 +302,16 @@ proc decodeSOF2(state: var DecoderState) = state.progressive = true proc reset(state: var DecoderState) = - ## Rests the decoder state need for reset markers. + ## Rests the decoder state need for restart markers. state.bitBuffer = 0 state.bitsBuffered = 0 for component in 0 ..< state.components.len: state.components[component].dcPred = 0 state.hitEnd = false if state.restartInterval != 0: - state.todo = state.restartInterval + state.todoBeforeRestart = state.restartInterval else: - state.todo = int.high + state.todoBeforeRestart = int.high state.eobRun = 0 proc decodeSOS(state: var DecoderState) = @@ -749,10 +749,10 @@ proc decodeBlock(state: var DecoderState, comp, row, column: int) = else: state.decodeRegularBlock(comp, data) -proc checkReset(state: var DecoderState) = - ## Check if we might have run into a reset marker, then deal with it. - dec state.todo - if state.todo <= 0: +proc checkRestart(state: var DecoderState) = + ## Check if we might have run into a restart marker, then deal with it. + dec state.todoBeforeRestart + if state.todoBeforeRestart <= 0: if state.bitsBuffered < 24: state.fillBitBuffer() @@ -760,7 +760,7 @@ proc checkReset(state: var DecoderState) = if state.buffer[state.pos + 1] in {0xD0.char .. 0xD7.char}: state.pos += 2 else: - failInvalid("did not get expected reset marker") + failInvalid("did not get expected restart marker") state.reset() proc decodeBlocks(state: var DecoderState) = @@ -774,7 +774,7 @@ proc decodeBlocks(state: var DecoderState) = for column in 0 ..< h: for row in 0 ..< w: state.decodeBlock(comp, row, column) - state.checkReset() + state.checkRestart() else: # Interleaved regular component pass. for mcuY in 0 ..< state.numMcuHigh: @@ -786,7 +786,7 @@ proc decodeBlocks(state: var DecoderState) = row = (mcuX * state.components[comp].yScale + compX) col = (mcuY * state.components[comp].xScale + compY) state.decodeBlock(comp, row, col) - state.checkReset() + state.checkRestart() proc quantizationAndIDCTPass(state: var DecoderState) = ## Does quantization and IDCT. @@ -932,8 +932,8 @@ proc decodeJpeg*(data: string): Image {.raises: [PixieError].} = # EOI - End of Image break of 0xD0 .. 0xD7: - # Reset markers - failInvalid("invalid reset marker") + # Restart markers + failInvalid("invalid restart marker") of 0xDB: # Define Quantization Table(s) state.decodeDQT() From 8036516c7d2d6b0107c5b3ade3d326a6621d20df Mon Sep 17 00:00:00 2001 From: Ryan Oldenburg Date: Mon, 16 May 2022 18:36:56 -0500 Subject: [PATCH 5/7] validate --- src/pixie/fileformats/jpeg.nim | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/pixie/fileformats/jpeg.nim b/src/pixie/fileformats/jpeg.nim index 54abc49..ad1ec8d 100644 --- a/src/pixie/fileformats/jpeg.nim +++ b/src/pixie/fileformats/jpeg.nim @@ -212,7 +212,7 @@ proc decodeDHT(state: var DecoderState) = proc decodeSOF0(state: var DecoderState) = ## Decode start of Frame - var len = state.readUint16be() - 2 + var len = state.readUint16be().int - 2 let precision = state.readUint8() if precision != 8: @@ -230,6 +230,8 @@ proc decodeSOF0(state: var DecoderState) = if numComponents notin {1, 3}: failInvalid("unsupported component count, must be 1 or 3") + len -= 6 + for i in 0 ..< numComponents: var component = Component() component.id = state.readUint8() @@ -250,6 +252,8 @@ proc decodeSOF0(state: var DecoderState) = component.quantizationTableId = quantizationTableId state.components.add(component) + len -= 3 * numComponents + for component in state.components.mitems: state.maxXScale = max(state.maxXScale, component.xScale) state.maxYScale = max(state.maxYScale, component.yScale) @@ -292,6 +296,9 @@ proc decodeSOF0(state: var DecoderState) = component.widthStride * component.heightStride ) + if len != 0: + failInvalid() + proc decodeSOF1(state: var DecoderState) = failInvalid("unsupported extended sequential DCT format") From e4e94b84f952e09887378b76c1b2b25df313d22a Mon Sep 17 00:00:00 2001 From: Ryan Oldenburg Date: Mon, 16 May 2022 18:41:49 -0500 Subject: [PATCH 6/7] f --- src/pixie/fileformats/jpeg.nim | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/pixie/fileformats/jpeg.nim b/src/pixie/fileformats/jpeg.nim index ad1ec8d..543c8ea 100644 --- a/src/pixie/fileformats/jpeg.nim +++ b/src/pixie/fileformats/jpeg.nim @@ -801,16 +801,14 @@ proc quantizationAndIDCTPass(state: var DecoderState) = let w = (state.components[comp].width + 7) shr 3 h = (state.components[comp].height + 7) shr 3 + qTableId = state.components[comp].quantizationTableId + if qTableId.int notin 0 ..< state.quantizationTables.len: + failInvalid() for column in 0 ..< h: for row in 0 ..< w: - var data = state.components[comp].blocks[row][column] - + var data {.byaddr.} = state.components[comp].blocks[row][column] for i in 0 ..< 64: - let qTableId = state.components[comp].quantizationTableId - if qTableId.int notin 0 ..< state.quantizationTables.len: - failInvalid() data[i] = cast[int16](data[i] * state.quantizationTables[qTableId][i].int32) - state.components[comp].idctBlock( state.components[comp].widthStride * column * 8 + row * 8, data @@ -900,8 +898,7 @@ proc buildImage(state: var DecoderState): Image = cr.unsafe[x, y], ) elif state.components.len == 1: - let - cy = state.components[0].channel + let cy = state.components[0].channel for y in 0 ..< state.imageHeight: for x in 0 ..< state.imageWidth: result.unsafe[x, y] = grayScaleToRgbx(cy.unsafe[x, y]) From 7afea49893078cfc624ee7f943c1155a4c31e58a Mon Sep 17 00:00:00 2001 From: Ryan Oldenburg Date: Mon, 16 May 2022 19:05:13 -0500 Subject: [PATCH 7/7] single branch --- src/pixie/fileformats/jpeg.nim | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/pixie/fileformats/jpeg.nim b/src/pixie/fileformats/jpeg.nim index 543c8ea..3bb484d 100644 --- a/src/pixie/fileformats/jpeg.nim +++ b/src/pixie/fileformats/jpeg.nim @@ -91,7 +91,11 @@ template failInvalid(reason = "unable to load") = template clampByte(x: int32): uint8 = ## Clamp integer into byte range. - clamp(x, 0, 0xFF).uint8 + # clamp(x, 0, 0xFF).uint8 + let + signBit = (cast[uint32](x) shr 31) + value = cast[uint32](x) and (signBit - 1) + min(value, 255).uint8 proc readUint8(state: var DecoderState): uint8 = ## Reads a byte from the input stream.