Fix possible buffer overflow in addMultirowsImg

[bptato]

In 4e46481, a workaround was applied to prevent buffer overruns in addMultirowsForm. Unfortunately, the same issue affects addMultirowsImg; I have been able to consistently reproduce it by enabling inline images and visiting https://html.spec.whatwg.org.

I have not yet figured out why exactly this can happen, but the start/end indices are calculated in the same way at both locations (i.e. at addMultirowsForm too), so it is unsurprising that the issue persists. For now, I've just copy-pasted the same bounds checks; further investigation might be needed on whether the COLPOS/columnPos functions themselves are faulty or they were just used incorrectly.

[rkta]

On Sat, Aug 05, 2023 at 01:14:03PM -0700, bptato wrote: In 4e46481, a workaround was applied to prevent buffer overruns in addMultirowsForm. Unfortunately, the same issue affects addMultirowsImg; I have been able to consistently reproduce it by enabling inline images and visiting https://html.spec.whatwg.org. I have not yet figured out why exactly this can happen, but the start/end indices are calculated in the same way at both locations (i.e. at addMultirowsForm too), so it is unsurprising that the issue persists. For now, I've just copy-pasted the same bounds checks; further investigation might be needed on whether the COLPOS/columnPos functions themselves are faulty or they were just used incorrectly.

I had a quick look. I can reproduce the issue, but also don't see why it happens. I tried with the input from the issues linked in the commit, to simplify the debugging but couldn't reproduce it with them (reverting the fix obviously). Maybe we should add a comment in the code explaining why we continue; that it's a band-aid patch and that the problem is likely with COLPOS/columnPos.

[bptato]

I've started to look into this a bit more. Some of my observations so far:

• The bounds check should be < 0, > l->size (so permit +1 on both sides).
  addMultirowsForm needs < 1, >= l->size because it then writes at the
  beginning/end of the bounds.
• For testing, I printed positions of the problematic images on a 105-col
  terminal window with ./configure --disable-m17n. Then I checked the line
  of the last image (26727). As it turns out, it's positioned incorrectly, and
  overlays the text of a completely unrelated paragraph (the image is a
  flowchart of the parser, and is drawn over the paragraph 4.8.4.4.15
  Guidance for conformance checkers).

From this my conclusion is that COLPOS/columnPos are not the real culprit,
but something is amiss in the image placement code that causes images to
be positioned on lines that may be shorter than required by the image
width.