kooloveme / thtmlviewer

Automatically exported from code.google.com/p/thtmlviewer
Other
0 stars 0 forks source link

Missing zero-termination can lead to GPF #162

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
The HTML buffer created by the SetStream method is not zero terminated which 
can lead to bogus crashes depending on memory layout (as this stream is passed 
to other routines expection zero termination in implicit string conversions). 
Suggested fix:

procedure TBuffer.SetStream(Stream: TStream);
var
  I: Integer;
begin
  I := Stream.Size - Stream.Position;
+++  SetLength(FBuffer, I + 1);
  //BG, 25.02.2011: Issue 74: Range check error when creating a buffer for the empty stream
  //  Do not read from empty/exhausted stream:
  if I > 0 then
+++    begin
    Stream.Read(FBuffer[0], I);
+++    FBuffer[i] := 0;
+++    end;
  Reset;
end;

Original issue reported on code.google.com by johnb20072@gmail.com on 25 Jun 2012 at 9:49

GoogleCodeExporter commented 9 years ago
John,

thanks for your suggestion.

Remember that FBuffer is a byte array, not a string.

Adding a trailing 0 increments the TBuffer.Size, which may confuse some Size 
related calculations.

Where did you get trouble with the buffer?

OrphanCat

Original comment by OrphanCat on 25 Jun 2012 at 5:09

GoogleCodeExporter commented 9 years ago
Not at the Office right now -  the buffer is used by a routine that determines 
the codepage of the text where it is cast to sth like PAnsiChar. From the 
disassembly, despite the char type this seems to expect zero termination. 
Applying the change fixed some rare crashes we were encountering. I could send 
more details tomorrow if this doesn't make sense to you. 

Original comment by johnb20072@gmail.com on 25 Jun 2012 at 5:27

GoogleCodeExporter commented 9 years ago
OK, the routine in question is TBuffer.DetectCodePage. The killing line in case 
of the GPF is

DocS := FPos.AnsiChr;

which internally runs into Delphi string cast routines that use zero 
termination to detect the required allocation length (compiled with XE2).

Original comment by johnb20072@gmail.com on 26 Jun 2012 at 9:15

GoogleCodeExporter commented 9 years ago
r309 fixes the issue in TBuffer.DetectCodePage.

Original comment by OrphanCat on 15 Jul 2012 at 4:24

GoogleCodeExporter commented 9 years ago
Works, thanks very much

Original comment by johnb20072@gmail.com on 19 Jul 2012 at 7:55