Fossil SCM

Missing UUID in manifest can crash manifest_parse()

Fixed

17d00c20dd9fa22… · opened 5 years, 7 months ago

Type
Code_Defect
Priority
Immediate
Severity
Important
Resolution
Fixed
Subsystem
Created
Aug. 17, 2020 5:26 p.m.

While testing a fix for it was discovered that an F-card with a name but no hash can trigger a null pointer dereference. To do this one has to create a semantically invalid manifest with a valid Z-card and add that to the blob table, then await the next rebuild or reconstruct. If the Z-card is invalid, that failure triggers before any other cards are parsed.

The demonstration below uses in F-card, but it can hypothetically be triggered by any card type which expects a hash token and gets no token instead.

e.g.

C switch/case\sstyle\stweak,\sper\srequest.
D 2020-08-17T15:40:00.105
F .dockerignore 29c5476ae4fb609219ea3f11d60c4b133a037b68
F .editorconfig 132c5a213aa3ce13dcc9c19f8a7ea306e3640bec4ae693378116cee339c34a1a
F .fossil-settings/binary-glob 34cc11b3b509fd5064afe327e70765799f314c9a
F .fossil-settings/clean-glob 50327fe7150c88e5daf5150f240dc805398665fb
F .fossil-settings/crlf-glob 2da052c2bd7a53c5e083152cd3ea3cefd6228b17
F .fossil-settings/encoding-glob fe9e7e2a183ce22d107976173b58d2a1255aeb0a
F .fossil-settings/ignore-glob b7b945d48cfceef7fd1000d6a6c6232b7163d5e8
F .project b7b33febfaccfeae87eb1d782cd0c675679b7dc7
F .settings/org.eclipse.core.resources.prefs 6d8a06b0a3395c74037897476eadb764d7f2d22d
F .settings/org.eclipse.core.runtime.prefs 6cc9381da1eff8e6dac0e41121d3cce3f1b4761b
F _FOSSIL_-sha <---- crash is here
F _FOSSIL_-shm <- must fail b/c ckout db name

Stacktrace:

0x00005555555bb4eb in validate16 (zIn=0x0, nIn=40) at ./src/encode.c:671
671   if( zIn[nIn]==0 ){
(gdb) bt
#0  0x00005555555bb4eb in validate16 (zIn=0x0, nIn=40) at ./src/encode.c:671
#1  0x00005555555d0b0e in hname_validate (zHash=zHash@entry=0x0, nHash=<optimized out>) at ./src/hname.c:87
#2  0x00005555555eceae in manifest_parse (pContent=0x5555559df9a0, pContent@entry=0x7fffffffdff0, rid=rid@entry=0, 
    pErr=pErr@entry=0x7fffffffe010) at ./src/manifest.c:641
#3  0x00005555555edbbd in manifest_test_parse_cmd () at ./src/manifest.c:1166
#4  0x00005555555e8768 in fossil_main (argc=<optimized out>, argv=<optimized out>) at ./src/main.c:938
#5  0x0000555555577ddf in main (argc=<optimized out>, argv=<optimized out>) at ./src/main.c:648

The solution is clearly to either add a NULL check to validate16() or (nicer) to trigger explicit syntax errors in the 12(?) manifest_parse() calls to hname_validate().

This will get fixed along with , once that fix is done.

Comments (2)

stephan 5 years, 7 months ago

While testing a fix for it was discovered that an F-card with a name but no hash can trigger a null pointer dereference. To do this one has to create a semantically invalid manifest with a valid Z-card and add that to the blob table, then await the next rebuild or reconstruct. If the Z-card is invalid, that failure triggers before any other cards are parsed.

The demonstration below uses in F-card, but it can hypothetically be triggered by any card type which expects a hash token and gets no token instead.

e.g.

C switch/case\sstyle\stweak,\sper\srequest.
D 2020-08-17T15:40:00.105
F .dockerignore 29c5476ae4fb609219ea3f11d60c4b133a037b68
F .editorconfig 132c5a213aa3ce13dcc9c19f8a7ea306e3640bec4ae693378116cee339c34a1a
F .fossil-settings/binary-glob 34cc11b3b509fd5064afe327e70765799f314c9a
F .fossil-settings/clean-glob 50327fe7150c88e5daf5150f240dc805398665fb
F .fossil-settings/crlf-glob 2da052c2bd7a53c5e083152cd3ea3cefd6228b17
F .fossil-settings/encoding-glob fe9e7e2a183ce22d107976173b58d2a1255aeb0a
F .fossil-settings/ignore-glob b7b945d48cfceef7fd1000d6a6c6232b7163d5e8
F .project b7b33febfaccfeae87eb1d782cd0c675679b7dc7
F .settings/org.eclipse.core.resources.prefs 6d8a06b0a3395c74037897476eadb764d7f2d22d
F .settings/org.eclipse.core.runtime.prefs 6cc9381da1eff8e6dac0e41121d3cce3f1b4761b
F _FOSSIL_-sha <---- crash is here
F _FOSSIL_-shm <- must fail b/c ckout db name

Stacktrace:

0x00005555555bb4eb in validate16 (zIn=0x0, nIn=40) at ./src/encode.c:671
671   if( zIn[nIn]==0 ){
(gdb) bt
#0  0x00005555555bb4eb in validate16 (zIn=0x0, nIn=40) at ./src/encode.c:671
#1  0x00005555555d0b0e in hname_validate (zHash=zHash@entry=0x0, nHash=<optimized out>) at ./src/hname.c:87
#2  0x00005555555eceae in manifest_parse (pContent=0x5555559df9a0, pContent@entry=0x7fffffffdff0, rid=rid@entry=0, 
    pErr=pErr@entry=0x7fffffffe010) at ./src/manifest.c:641
#3  0x00005555555edbbd in manifest_test_parse_cmd () at ./src/manifest.c:1166
#4  0x00005555555e8768 in fossil_main (argc=<optimized out>, argv=<optimized out>) at ./src/main.c:938
#5  0x0000555555577ddf in main (argc=<optimized out>, argv=<optimized out>) at ./src/main.c:648

The solution is clearly to either add a NULL check to validate16() or (nicer) to trigger explicit syntax errors in the 12(?) manifest_parse() calls to hname_validate().

This will get fixed along with , once that fix is done.

stephan 5 years, 7 months ago

Fixed in . It turned out that only the E- and F-cards were missing that particular validation.

Keyboard Shortcuts

Open search /
Next entry (timeline) j
Previous entry (timeline) k
Open focused entry Enter
Show this help ?
Toggle theme Top nav button