Skip to content

Commit

Permalink
pagekitec: add patch to fix use after free
Browse files Browse the repository at this point in the history
Observation:
- programs using libpagekitec did crash with SIGSEGV on startup on RPi3,4
  while having worked fine for years on RPi1+2

Explantation:
- the final "judgement" test were done on pointers into the `copy` buffer
  freed on line 766 instead of on the safe copies of those strings in `kite`
  and `kite_r`.
- this opened a very short race condition window, however the crash was
  caught happening while a tight loop (`pkb_start_blockers`) fired up 16
  of those threads in rapid succession.
  So probably if the next thread got to allocate memory before the
  "judgements" tests, accessing the just freed `copy` would cause a segfault.

Fix:
- check the safe copies of the strings instead of pointers into freed `copy`.

Signed-off-by: Lukas Zeller <[email protected]>
  • Loading branch information
plan44 committed Sep 17, 2024
1 parent 180046b commit ed379e5
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 1 deletion.
2 changes: 1 addition & 1 deletion net/pagekitec/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ include $(TOPDIR)/rules.mk
PKG_NAME:=pagekitec
PKG_REV:=0.91.201110
PKG_VERSION:=$(PKG_REV)C
PKG_RELEASE:=2
PKG_RELEASE:=3
PKG_LICENSE:=Apache-2.0
PKG_SOURCE_SUBDIR:=$(PKG_NAME)-$(PKG_VERSION)
PKG_SOURCE:=$(PKG_NAME)-$(PKG_VERSION).tar.xz
Expand Down
45 changes: 45 additions & 0 deletions net/pagekitec/patches/0003-fix-use-after-free-in-pkproto.c.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
From ef75e4d5e517ed12d6057b2211d401fa1ce84f4a Mon Sep 17 00:00:00 2001
From: Lukas Zeller <[email protected]>
Date: Tue, 17 Sep 2024 00:53:40 +0200
Subject: [PATCH] pkproto.c: fix use-after-free that did cause pagekite to
SIGSEGV occasionally, with higher probability on faster/multicore systems

Observation:
- programs using libpagekitec did crash with SIGSEGV on startup on RPi3,4
while having worked fine for years on RPi1+2

Explantation:
- the final "judgement" test were done on pointers into the `copy` buffer
freed on line 766 instead of on the safe copies of those strings in `kite`
and `kite_r`.
- this opened a very short race condition window, however the crash was
caught happening while a tight loop (`pkb_start_blockers`) fired up 16
of those threads in rapid succession.
So probably if the next thread got to allocate memory before the
"judgements" tests, accessing the just freed `copy` would cause a segfault.

Fix:
- check the safe copies of the strings instead of pointers into freed `copy`.

Signed-off-by: Lukas Zeller <[email protected]>
---
libpagekite/pkproto.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/libpagekite/pkproto.c b/libpagekite/pkproto.c
index 3d4ccd6..4ee7e0f 100644
--- a/libpagekite/pkproto.c
+++ b/libpagekite/pkproto.c
@@ -766,9 +766,9 @@ char *pk_parse_kite_request(
free(copy);

/* Pass judgement */
- if ('\0' == *public_domain) return pk_err_null(ERR_PARSE_NO_KITENAME);
- if ('\0' == *bsalt) return pk_err_null(ERR_PARSE_NO_BSALT);
- if ('\0' == *fsalt) return pk_err_null(ERR_PARSE_NO_FSALT);
+ if ('\0' == *(kite->public_domain)) return pk_err_null(ERR_PARSE_NO_KITENAME);
+ if ('\0' == *(kite_r->bsalt)) return pk_err_null(ERR_PARSE_NO_BSALT);
+ if ('\0' == *(kite_r->fsalt)) return pk_err_null(ERR_PARSE_NO_FSALT);
return kite->public_domain;
}

0 comments on commit ed379e5

Please sign in to comment.