Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crash in html.go found from fuzzing #13819

Closed
AdamKorcz opened this issue Dec 3, 2020 · 3 comments · Fixed by #13839
Closed

Crash in html.go found from fuzzing #13819

AdamKorcz opened this issue Dec 3, 2020 · 3 comments · Fixed by #13839
Labels

Comments

@AdamKorcz
Copy link
Contributor

This is a bug report for a crash found with the PostProcess fuzzer from here.

The version of gitea is the the master branch downloaded with git clone https://github.com/go-gitea/gitea

Stacktrace:

goroutine 17 [running, locked to thread]:
code.gitea.io/gitea/modules/markup.shortLinkProcessorFull(0x10c001189f80, 0x10c000341500, 0x0)
        code.gitea.io/gitea/modules/markup/html.go:660 +0x25e5
code.gitea.io/gitea/modules/markup.shortLinkProcessor(0x10c001189f80, 0x10c000341500)
        code.gitea.io/gitea/modules/markup/html.go:607 +0x4b
code.gitea.io/gitea/modules/markup.(*postProcessCtx).textNode(0x10c001189f80, 0x10c000341500)
        code.gitea.io/gitea/modules/markup/html.go:440 +0x7d
code.gitea.io/gitea/modules/markup.(*postProcessCtx).visitNode(0x10c001189f80, 0x10c000341500, 0x7fe4b1d20101)
        code.gitea.io/gitea/modules/markup/html.go:380 +0x1525
code.gitea.io/gitea/modules/markup.(*postProcessCtx).visitNode(0x10c001189f80, 0x10c000341420, 0x1)
        code.gitea.io/gitea/modules/markup/html.go:430 +0x66c
code.gitea.io/gitea/modules/markup.(*postProcessCtx).visitNode(0x10c001189f80, 0x10c000341340, 0x1)
        code.gitea.io/gitea/modules/markup/html.go:430 +0x66c
code.gitea.io/gitea/modules/markup.(*postProcessCtx).postProcess(0x10c001189f80, 0x60800001bea0, 0x60, 0x60, 0x683286c0dcf93c66, 0x10c001edddd8, 0x56422f, 0x1725480, 0x10c0006a4360)
        code.gitea.io/gitea/modules/markup/html.go:341 +0x305
code.gitea.io/gitea/modules/markup.PostProcess(0x60800001bea0, 0x60, 0x60, 0xf1f147, 0x4, 0x10c000675f20, 0x0, 0x10c0001d2c59, 0x0, 0x56a394, ...)
        code.gitea.io/gitea/modules/markup/html.go:193 +0xdd
code.gitea.io/gitea/fuzz.Fuzz2(0x60800001bea0, 0x60, 0x60, 0x18)
        code.gitea.io/gitea/fuzz/fuzz.go:24 +0x12f
main.LLVMFuzzerTestOneInput(...)
        command-line-arguments/main.711778661.go:21
main._cgoexpwrap_441dadf043a3_LLVMFuzzerTestOneInput(0x60800001bea0, 0x60, 0x1e6d000)
        _cgo_gotypes.go:53 +0x5c
AddressSanitizer:DEADLYSIGNAL
=================================================================
==11==ERROR: AddressSanitizer: ABRT on unknown address 0x00000000000b (pc 0x0000005c1481 bp 0x10c001edcf90 sp 0x10c001edcf78 T0)
SCARINESS: 10 (signal)
    #0 0x5c1481 in runtime.raise runtime/sys_linux_amd64.s:165

DEDUP_TOKEN: runtime.raise
AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: ABRT runtime/sys_linux_amd64.s:165 in runtime.raise
==11==ABORTING

Input buffer:

0x52,0xfe,0xff,0x0,0x0,0x44,0x1,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x5b,0x5b,0x30,0x5b,0x3a,0x3a,0x3a,0x3d,0x0,0x27,0x0,0x5d,0x5d,0x21,0x5d,0x3a,0x21,0x0,0xd1,0x82,0x0,0x44,0x1,0x0,0x0,0x0,0x6f,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x5b,0x5b,0x30,0x5b,0x3a,0x3a,0x3d,0x27,0x3a,0x8,0x0,0x0,0x0,0x80,0x5d,0x5d,0x5d,0x21,0x5d,0x3a,0x21,0x0,0xd1,0x82,0x0,0xcc,0x5b,0x3c,0x3c,0x5b,0x31,0x5b,0x7c,0x3c,0x3c,0x3a,0x3a,0x5b,0x5b,0xdf,0x0

or with ASCII characters:

R\xfe\xff\x00\x00D\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00[[0[:::=\x00'\x00]]!]:!\x00\xd1\x82\x00D\x01\x00\x00\x00o\x00\x00\x00\x00\x00\x00\x00\x00\x00[[0[::=':\x08\x00\x00\x00\x80]]]!]:!\x00\xd1\x82\x00\xcc[<<[1[|<<::[[\xdf\x00
@lunny lunny added the type/bug label Dec 3, 2020
@zeripath
Copy link
Contributor

zeripath commented Dec 4, 2020

So this causes a panic which should be caught and an error page shown.

Looking at the code:

} else if (strings.HasPrefix(val, "\"") && strings.HasSuffix(val, "\"")) ||
(strings.HasPrefix(val, "'") && strings.HasSuffix(val, "'")) {
val = val[1 : len(val)-1]

To get a panic here the length of val at this point has to be only 1 and either equal to " or '.

Thus a much simpler reproducible error is:

[[a|b=']]

E.g. https://try.gitea.io/arandomer/pathological/raw/branch/master/Another.md

https://try.gitea.io/arandomer/pathological/src/branch/master/Another.md

Now it looks like as a result of the chi pr the panic catcher has been broken as that panic should have and would have been caught by macaron previously and hidden showing a 500...

@zeripath
Copy link
Contributor

zeripath commented Dec 4, 2020

Lines 651-664 or simply 658 should probably just get a Len check on them - a similar panic would be possible with [[a|b="]]

@zeripath
Copy link
Contributor

zeripath commented Dec 4, 2020

Mea culpa - I added this and totally missed that ' would pass both tests. A simple Len check would definitely prevent this and is likely to be the cheapest solution.

mrsdizzie added a commit to mrsdizzie/gitea that referenced this issue Dec 4, 2020
6543 pushed a commit that referenced this issue Dec 4, 2020
mrsdizzie added a commit to mrsdizzie/gitea that referenced this issue Dec 4, 2020
6543 pushed a commit that referenced this issue Dec 4, 2020
@go-gitea go-gitea locked and limited conversation to collaborators Jan 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants