Fossil SCM
Missing UUID in manifest can crash manifest_parse()
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().
Comments (2)
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().