~rjarry/aerc-devel

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
2 2

[PATCH aerc] composer: check bounds of composer.Focus method

Details
Message ID
<20220923162643.70878-1-tim@timculverhouse.com>
DKIM signature
pass
Download raw message
Patch: +3 -0
The composer sets focus of a widget via a slice with direct access via
an index. Protect the bounds of the slice before calling a method on it
to prevent a panic.

Reported panic:
                                  PANIC CAUGHT!
                        2022-09-23T16:56:48.838457+0200

aerc has encountered a critical error and has terminated. Please help us fix
this by sending this log and the steps to reproduce the crash to:
~rjarry/aerc-devel@lists.sr.ht

Thank you

Version: 0.12.0-42-g9fdc7acf5b48 +notmuch (go1.18.4 amd64 linux)
Error: runtime error: index out of range [4] with length 4

goroutine 11 [running]:
runtime/debug.Stack()
	runtime/debug/stack.go:24 +0x65
git.sr.ht/~rjarry/aerc/logging.PanicHandler()
	git.sr.ht/~rjarry/aerc/logging/panic-logger.go:49 +0x6af
panic({0xa82460, 0xc0053bcdb0})
	runtime/panic.go:838 +0x207
git.sr.ht/~rjarry/aerc/widgets.(*Composer).Focus(0xc0000efe30?, 0x0?)
	git.sr.ht/~rjarry/aerc/widgets/compose.go:618 +0x51
git.sr.ht/~rjarry/aerc/widgets.(*Aerc).focus(0xc000350000, {0x0?, 0x0})
	git.sr.ht/~rjarry/aerc/widgets/aerc.go:568 +0xec
git.sr.ht/~rjarry/aerc/widgets.(*Aerc).BeginExCommand.func2()
	git.sr.ht/~rjarry/aerc/widgets/aerc.go:590 +0x4c
git.sr.ht/~rjarry/aerc/widgets.(*ExLine).Event(0xc001d43e50, {0xbb75e0?, 0xc00995a5c0?})
	git.sr.ht/~rjarry/aerc/widgets/exline.go:81 +0xbc
git.sr.ht/~rjarry/aerc/widgets.(*Aerc).Event(0xc002a94fa0?, {0xbb75e0?, 0xc00995a5c0?})
	git.sr.ht/~rjarry/aerc/widgets/aerc.go:285 +0x470
git.sr.ht/~rjarry/aerc/lib/ui.(*UI).ProcessEvents(0xc0004b20f0)
	git.sr.ht/~rjarry/aerc/lib/ui/ui.go:117 +0x202
created by main.main
	git.sr.ht/~rjarry/aerc/aerc.go:244 +0x94c

Reported-by: Bence Ferdinandy <bence@ferdinandy.com>
Signed-off-by: Tim Culverhouse <tim@timculverhouse.com>
---
 widgets/compose.go | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/widgets/compose.go b/widgets/compose.go
index c0740d2241aa..69cc4f84fe6e 100644
--- a/widgets/compose.go
+++ b/widgets/compose.go
@@ -615,6 +615,9 @@ func (c *Composer) MouseEvent(localX int, localY int, event tcell.Event) {
}

func (c *Composer) Focus(focus bool) {
	if c.focused > len(c.focusable) {
		c.focused = len(c.focusable)
	}
	c.focusable[c.focused].Focus(focus)
}

-- 
2.37.3

[aerc/patches] build success

builds.sr.ht <builds@sr.ht>
Details
Message ID
<CN3X9Y660A3Q.30Q34L1QP5ZF@cirno2>
In-Reply-To
<20220923162643.70878-1-tim@timculverhouse.com> (view parent)
DKIM signature
missing
Download raw message
aerc/patches: SUCCESS in 3m54s

[composer: check bounds of composer.Focus method][0] from [Tim Culverhouse][1]

[0]: https://lists.sr.ht/~rjarry/aerc-devel/patches/35533
[1]: tim@timculverhouse.com

✓ #849687 SUCCESS aerc/patches/openbsd.yml     https://builds.sr.ht/~rjarry/job/849687
✓ #849686 SUCCESS aerc/patches/alpine-edge.yml https://builds.sr.ht/~rjarry/job/849686
Details
Message ID
<CN3YB6U4E1AH.2SYDGMCJ8JNQM@marty>
In-Reply-To
<20220923162643.70878-1-tim@timculverhouse.com> (view parent)
DKIM signature
pass
Download raw message
Tim Culverhouse, Sep 23, 2022 at 18:26:
> The composer sets focus of a widget via a slice with direct access via
> an index. Protect the bounds of the slice before calling a method on it
> to prevent a panic.
>
> Reported panic:
>                                   PANIC CAUGHT!
>                         2022-09-23T16:56:48.838457+0200
>
> aerc has encountered a critical error and has terminated. Please help us fix
> this by sending this log and the steps to reproduce the crash to:
> ~rjarry/aerc-devel@lists.sr.ht
>
> Thank you
>
> Version: 0.12.0-42-g9fdc7acf5b48 +notmuch (go1.18.4 amd64 linux)
> Error: runtime error: index out of range [4] with length 4
>
> goroutine 11 [running]:
> runtime/debug.Stack()
> 	runtime/debug/stack.go:24 +0x65
> git.sr.ht/~rjarry/aerc/logging.PanicHandler()
> 	git.sr.ht/~rjarry/aerc/logging/panic-logger.go:49 +0x6af
> panic({0xa82460, 0xc0053bcdb0})
> 	runtime/panic.go:838 +0x207
> git.sr.ht/~rjarry/aerc/widgets.(*Composer).Focus(0xc0000efe30?, 0x0?)
> 	git.sr.ht/~rjarry/aerc/widgets/compose.go:618 +0x51
> git.sr.ht/~rjarry/aerc/widgets.(*Aerc).focus(0xc000350000, {0x0?, 0x0})
> 	git.sr.ht/~rjarry/aerc/widgets/aerc.go:568 +0xec
> git.sr.ht/~rjarry/aerc/widgets.(*Aerc).BeginExCommand.func2()
> 	git.sr.ht/~rjarry/aerc/widgets/aerc.go:590 +0x4c
> git.sr.ht/~rjarry/aerc/widgets.(*ExLine).Event(0xc001d43e50, {0xbb75e0?, 0xc00995a5c0?})
> 	git.sr.ht/~rjarry/aerc/widgets/exline.go:81 +0xbc
> git.sr.ht/~rjarry/aerc/widgets.(*Aerc).Event(0xc002a94fa0?, {0xbb75e0?, 0xc00995a5c0?})
> 	git.sr.ht/~rjarry/aerc/widgets/aerc.go:285 +0x470
> git.sr.ht/~rjarry/aerc/lib/ui.(*UI).ProcessEvents(0xc0004b20f0)
> 	git.sr.ht/~rjarry/aerc/lib/ui/ui.go:117 +0x202
> created by main.main
> 	git.sr.ht/~rjarry/aerc/aerc.go:244 +0x94c
>
> Reported-by: Bence Ferdinandy <bence@ferdinandy.com>
> Signed-off-by: Tim Culverhouse <tim@timculverhouse.com>
> ---
>  widgets/compose.go | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/widgets/compose.go b/widgets/compose.go
> index c0740d2241aa..69cc4f84fe6e 100644
> --- a/widgets/compose.go
> +++ b/widgets/compose.go
> @@ -615,6 +615,9 @@ func (c *Composer) MouseEvent(localX int, localY int, event tcell.Event) {
>  }
>  
>  func (c *Composer) Focus(focus bool) {
> +	if c.focused > len(c.focusable) {
> +		c.focused = len(c.focusable)
> +	}
>  	c.focusable[c.focused].Focus(focus)

This is inherently racy. Nothing guarantees that c.focusable will not be
modified after checking its length but before accessing it.

If there is an issue here, we need to protect concurrent accesses with
a lock.
Reply to thread Export thread (mbox)