At the moment several procedures can fail at allocating. But there use do not check if they failed.
kbts__InputCodepoint kbts__FindOrCreateShapeConfig kbts__PointerPushType
Activity
3
JimmyLefevre4mo agoI have improved some things with kbts_InputCodepoint and kbts_FindOrCreateShapeConfig:
https://github.com/JimmyLefevre/kb/commit/3ddf22dcff112e93ded62f8eba867dc4f3a66f68
I'd be willing to look at specific issues if you have any, otherwise this seems fine to me?
dimenus18d agoHere is a specific one. Tested with commit cc63806 (v2.28e).
When kbts__NewBucketedGlyphBlock returns NULL, kbts__InsertGlyphIntoBucket still writes Block->Glyphs[Block->Count++] into the last block, which is already full:
if(NeedNewBlock)
{
kbts__bucketed_glyph_block *NewBlock = kbts__NewBucketedGlyphBlock(Scratchpad);
if(NewBlock)
{
...
}
}
kbts__bucketed_glyph_block *Block = (kbts__bucketed_glyph_block *)Header;
kbts__bucketed_glyph *Result = &Block->Glyphs[Block->Count++];So an allocator that returns NULL causes a heap write past the end of a block. This repro uses an allocator that refuses every request after 64 MB, and TestGSUBThree.ttf from HarfBuzz's tests, whose GSUB grows "lol" without limit:
// cc -g -fsanitize=address oom.c -o oom && ./oom TestGSUBThree.ttf
// An allocator that refuses every request once 64 MB are in use.
#define KB_TEXT_SHAPE_IMPLEMENTATION
#include "kb_text_shape.h"
#include "common.h"
#include <string.h>
static size_t used;
static void limited_allocator(void *data, kbts_allocator_op *op)
{
(void)data;
if (op->Kind == KBTS_ALLOCATOR_OP_KIND_ALLOCATE) {
size_t size = op->Allocate.Size;
op->Allocate.Pointer = 0;
if (used + size > ((size_t)64 << 20)) { return; }
char *block = malloc(size + 16);
memcpy(block, &size, sizeof(size));
used += size;
op->Allocate.Pointer = block + 16;
} else if (op->Kind == KBTS_ALLOCATOR_OP_KIND_FREE && op->Free.Pointer) {
char *block = (char *)op->Free.Pointer - 16;
size_t size;
memcpy(&size, block, sizeof(size));
used -= size;
free(block);
}
}
int main(int argc, char **argv)
{
kbts_shape_context *context = kbts_CreateShapeContext(limited_allocator, 0);
if (!kbts_ShapePushFontFromFile(context, argv[1], 0)) { printf("font did not load\n"); return 1; }
kbts_ShapeBegin(context, KBTS_DIRECTION_DONT_KNOW, KBTS_LANGUAGE_DONT_KNOW);
kbts_ShapeUtf8(context, "lol", 3, KBTS_USER_ID_GENERATION_MODE_CODEPOINT_INDEX);
kbts_ShapeEnd(context);
long glyphs = 0;
kbts_run run;
while (kbts_ShapeRun(context, &run)) {
kbts_glyph *glyph;
while (kbts_GlyphIteratorNext(&run.Glyphs, &glyph)) { glyphs++; }
}
printf("glyphs: %ld\n", glyphs);
printf("kbts_ShapeError: %d\n", (int)kbts_ShapeError(context));
return 0;
}==ERROR: AddressSanitizer: heap-buffer-overflow
WRITE of size 4 thread T0
#0 kbts__InsertGlyphIntoBucket kb_text_shape.h:18861
#1 kbts__BucketGlyph kb_text_shape.h:18909
#2 kbts__GsubMutate kb_text_shape.h:18979
#3 kbts__DoSubstitution kb_text_shape.h:20700
#4 kbts__ExecuteOp kb_text_shape.h:22104
#5 kbts__ShapeDirect kb_text_shape.h:26802
0x621004ff3510 is located 0 bytes after 4112-byte regionA smaller one in the same area: kbts__CreateShapeScratchpad writes Result->SelfAllocated = 1 without checking whether the allocation succeeded.
(I found this with help from Claude, with a harness that runs HarfBuzz's shaping test suite through kb and through HarfBuzz and compares them. I checked the repro by hand.)
I have improved some things with
kbts_InputCodepointandkbts_FindOrCreateShapeConfig: https://github.com/JimmyLefevre/kb/commit/3ddf22dcff112e93ded62f8eba867dc4f3a66f68I'd be willing to look at specific issues if you have any, otherwise this seems fine to me?