[OE-core][kirkstone][PATCH v2] shadow: backport patch to fix CVE-2023-29383

[OE-core][kirkstone][PATCH v2] shadow: backport patch to fix CVE-2023-29383

[Xiangyu Chen]

From: Xiangyu Chen <xiangyu.chen@windriver.com>

The fix of CVE-2023-29383.patch contains a bug that it rejects all
characters that are not control ones, so backup another patch named
"0001-Overhaul-valid_field.patch" from upstream to fix it.

Signed-off-by: Xiangyu Chen <xiangyu.chen@windriver.com>
---
Changes

v1->v2: 
1. Based on latest oe-core commit
2. The fix of cve caused useradd/groupadd report errors as below:
"configuration error - unknown item 'SYSLOG_SU_ENAB' (notify administrator)"
"configuration error - unknown item 'SYSLOG_SG_ENAB' (notify administrator)"
so backport another patch to fix useradd/groupadd wrong paramter's issue.
3. Using CVE-xxx as fix of cve patch file name
---
 .../files/0001-Overhaul-valid_field.patch     | 65 +++++++++++++++++++
 .../shadow/files/CVE-2023-29383.patch         | 53 +++++++++++++++
 meta/recipes-extended/shadow/shadow.inc       |  2 +
 3 files changed, 120 insertions(+)
 create mode 100644 meta/recipes-extended/shadow/files/0001-Overhaul-valid_field.patch
 create mode 100644 meta/recipes-extended/shadow/files/CVE-2023-29383.patch

diff --git a/meta/recipes-extended/shadow/files/0001-Overhaul-valid_field.patch b/meta/recipes-extended/shadow/files/0001-Overhaul-valid_field.patch
new file mode 100644
index 0000000000..ac08be515b
--- /dev/null
+++ b/meta/recipes-extended/shadow/files/0001-Overhaul-valid_field.patch
@@ -0,0 +1,65 @@
+From 2eaea70111f65b16d55998386e4ceb4273c19eb4 Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com>
+Date: Fri, 31 Mar 2023 14:46:50 +0200
+Subject: [PATCH] Overhaul valid_field()
+
+e5905c4b ("Added control character check") introduced checking for
+control characters but had the logic inverted, so it rejects all
+characters that are not control ones.
+
+Cast the character to `unsigned char` before passing to the character
+checking functions to avoid UB.
+
+Use strpbrk(3) for the illegal character test and return early.
+
+Upstream-Status: Backport [https://github.com/shadow-maint/shadow/commit/2eaea70111f65b16d55998386e4ceb4273c19eb4]
+
+Signed-off-by: Xiangyu Chen <xiangyu.chen@windriver.com>
+---
+ lib/fields.c | 24 ++++++++++--------------
+ 1 file changed, 10 insertions(+), 14 deletions(-)
+
+diff --git a/lib/fields.c b/lib/fields.c
+index fb51b582..53929248 100644
+--- a/lib/fields.c
++++ b/lib/fields.c
+@@ -37,26 +37,22 @@ int valid_field (const char *field, const char *illegal)
+ 
+ 	/* For each character of field, search if it appears in the list
+ 	 * of illegal characters. */
++	if (illegal && NULL != strpbrk (field, illegal)) {
++		return -1;
++	}
++
++	/* Search if there are non-printable or control characters */
+ 	for (cp = field; '\0' != *cp; cp++) {
+-		if (strchr (illegal, *cp) != NULL) {
++		unsigned char c = *cp;
++		if (!isprint (c)) {
++			err = 1;
++		}
++		if (iscntrl (c)) {
+ 			err = -1;
+ 			break;
+ 		}
+ 	}
+ 
+-	if (0 == err) {
+-		/* Search if there are non-printable or control characters */
+-		for (cp = field; '\0' != *cp; cp++) {
+-			if (!isprint (*cp)) {
+-				err = 1;
+-			}
+-			if (!iscntrl (*cp)) {
+-				err = -1;
+-				break;
+-			}
+-		}
+-	}
+-
+ 	return err;
+ }
+ 
+-- 
+2.34.1
+
diff --git a/meta/recipes-extended/shadow/files/CVE-2023-29383.patch b/meta/recipes-extended/shadow/files/CVE-2023-29383.patch
new file mode 100644
index 0000000000..f53341d3fc
--- /dev/null
+++ b/meta/recipes-extended/shadow/files/CVE-2023-29383.patch
@@ -0,0 +1,53 @@
+From e5905c4b84d4fb90aefcd96ee618411ebfac663d Mon Sep 17 00:00:00 2001
+From: tomspiderlabs <128755403+tomspiderlabs@users.noreply.github.com>
+Date: Thu, 23 Mar 2023 23:39:38 +0000
+Subject: [PATCH] Added control character check
+
+Added control character check, returning -1 (to "err") if control characters are present.
+
+CVE: CVE-2023-29383
+Upstream-Status: Backport
+
+Reference to upstream:
+https://github.com/shadow-maint/shadow/commit/e5905c4b84d4fb90aefcd96ee618411ebfac663d
+
+Signed-off-by: Xiangyu Chen <xiangyu.chen@windriver.com>
+---
+ lib/fields.c | 11 +++++++----
+ 1 file changed, 7 insertions(+), 4 deletions(-)
+
+diff --git a/lib/fields.c b/lib/fields.c
+index 640be931..fb51b582 100644
+--- a/lib/fields.c
++++ b/lib/fields.c
+@@ -21,9 +21,9 @@
+  *
+  * The supplied field is scanned for non-printable and other illegal
+  * characters.
+- *  + -1 is returned if an illegal character is present.
+- *  +  1 is returned if no illegal characters are present, but the field
+- *       contains a non-printable character.
++ *  + -1 is returned if an illegal or control character is present.
++ *  +  1 is returned if no illegal or control characters are present,
++ *       but the field contains a non-printable character.
+  *  +  0 is returned otherwise.
+  */
+ int valid_field (const char *field, const char *illegal)
+@@ -45,10 +45,13 @@ int valid_field (const char *field, const char *illegal)
+ 	}
+ 
+ 	if (0 == err) {
+-		/* Search if there are some non-printable characters */
++		/* Search if there are non-printable or control characters */
+ 		for (cp = field; '\0' != *cp; cp++) {
+ 			if (!isprint (*cp)) {
+ 				err = 1;
++			}
++			if (!iscntrl (*cp)) {
++				err = -1;
+ 				break;
+ 			}
+ 		}
+-- 
+2.34.1
+
diff --git a/meta/recipes-extended/shadow/shadow.inc b/meta/recipes-extended/shadow/shadow.inc
index 5106b95571..3c1dd2f98e 100644
--- a/meta/recipes-extended/shadow/shadow.inc
+++ b/meta/recipes-extended/shadow/shadow.inc
@@ -16,6 +16,8 @@ SRC_URI = "https://github.com/shadow-maint/shadow/releases/download/v${PV}/${BP}
            ${@bb.utils.contains('PACKAGECONFIG', 'pam', '${PAM_SRC_URI}', '', d)} \
            file://shadow-relaxed-usernames.patch \
            file://useradd \
+           file://CVE-2023-29383.patch \
+           file://0001-Overhaul-valid_field.patch \
            "
 
 SRC_URI:append:class-target = " \
-- 
2.34.1

Re: [OE-core][kirkstone][PATCH v2] shadow: backport patch to fix CVE-2023-29383

[Steve Sakoman]

On Tue, Apr 18, 2023 at 8:18 PM Xiangyu Chen
<xiangyu.chen@eng.windriver.com> wrote:
>
> From: Xiangyu Chen <xiangyu.chen@windriver.com>
>
> The fix of CVE-2023-29383.patch contains a bug that it rejects all
> characters that are not control ones, so backup another patch named
> "0001-Overhaul-valid_field.patch" from upstream to fix it.
>
> Signed-off-by: Xiangyu Chen <xiangyu.chen@windriver.com>
> ---
> Changes
>
> v1->v2:
> 1. Based on latest oe-core commit
> 2. The fix of cve caused useradd/groupadd report errors as below:
> "configuration error - unknown item 'SYSLOG_SU_ENAB' (notify administrator)"
> "configuration error - unknown item 'SYSLOG_SG_ENAB' (notify administrator)"
> so backport another patch to fix useradd/groupadd wrong paramter's issue.
> 3. Using CVE-xxx as fix of cve patch file name

Thanks!  V2 looks much better :-)

Steve

> ---
>  .../files/0001-Overhaul-valid_field.patch     | 65 +++++++++++++++++++
>  .../shadow/files/CVE-2023-29383.patch         | 53 +++++++++++++++
>  meta/recipes-extended/shadow/shadow.inc       |  2 +
>  3 files changed, 120 insertions(+)
>  create mode 100644 meta/recipes-extended/shadow/files/0001-Overhaul-valid_field.patch
>  create mode 100644 meta/recipes-extended/shadow/files/CVE-2023-29383.patch
>
> diff --git a/meta/recipes-extended/shadow/files/0001-Overhaul-valid_field.patch b/meta/recipes-extended/shadow/files/0001-Overhaul-valid_field.patch
> new file mode 100644
> index 0000000000..ac08be515b
> --- /dev/null
> +++ b/meta/recipes-extended/shadow/files/0001-Overhaul-valid_field.patch
> @@ -0,0 +1,65 @@
> +From 2eaea70111f65b16d55998386e4ceb4273c19eb4 Mon Sep 17 00:00:00 2001
> +From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com>
> +Date: Fri, 31 Mar 2023 14:46:50 +0200
> +Subject: [PATCH] Overhaul valid_field()
> +
> +e5905c4b ("Added control character check") introduced checking for
> +control characters but had the logic inverted, so it rejects all
> +characters that are not control ones.
> +
> +Cast the character to `unsigned char` before passing to the character
> +checking functions to avoid UB.
> +
> +Use strpbrk(3) for the illegal character test and return early.
> +
> +Upstream-Status: Backport [https://github.com/shadow-maint/shadow/commit/2eaea70111f65b16d55998386e4ceb4273c19eb4]
> +
> +Signed-off-by: Xiangyu Chen <xiangyu.chen@windriver.com>
> +---
> + lib/fields.c | 24 ++++++++++--------------
> + 1 file changed, 10 insertions(+), 14 deletions(-)
> +
> +diff --git a/lib/fields.c b/lib/fields.c
> +index fb51b582..53929248 100644
> +--- a/lib/fields.c
> ++++ b/lib/fields.c
> +@@ -37,26 +37,22 @@ int valid_field (const char *field, const char *illegal)
> +
> +       /* For each character of field, search if it appears in the list
> +        * of illegal characters. */
> ++      if (illegal && NULL != strpbrk (field, illegal)) {
> ++              return -1;
> ++      }
> ++
> ++      /* Search if there are non-printable or control characters */
> +       for (cp = field; '\0' != *cp; cp++) {
> +-              if (strchr (illegal, *cp) != NULL) {
> ++              unsigned char c = *cp;
> ++              if (!isprint (c)) {
> ++                      err = 1;
> ++              }
> ++              if (iscntrl (c)) {
> +                       err = -1;
> +                       break;
> +               }
> +       }
> +
> +-      if (0 == err) {
> +-              /* Search if there are non-printable or control characters */
> +-              for (cp = field; '\0' != *cp; cp++) {
> +-                      if (!isprint (*cp)) {
> +-                              err = 1;
> +-                      }
> +-                      if (!iscntrl (*cp)) {
> +-                              err = -1;
> +-                              break;
> +-                      }
> +-              }
> +-      }
> +-
> +       return err;
> + }
> +
> +--
> +2.34.1
> +
> diff --git a/meta/recipes-extended/shadow/files/CVE-2023-29383.patch b/meta/recipes-extended/shadow/files/CVE-2023-29383.patch
> new file mode 100644
> index 0000000000..f53341d3fc
> --- /dev/null
> +++ b/meta/recipes-extended/shadow/files/CVE-2023-29383.patch
> @@ -0,0 +1,53 @@
> +From e5905c4b84d4fb90aefcd96ee618411ebfac663d Mon Sep 17 00:00:00 2001
> +From: tomspiderlabs <128755403+tomspiderlabs@users.noreply.github.com>
> +Date: Thu, 23 Mar 2023 23:39:38 +0000
> +Subject: [PATCH] Added control character check
> +
> +Added control character check, returning -1 (to "err") if control characters are present.
> +
> +CVE: CVE-2023-29383
> +Upstream-Status: Backport
> +
> +Reference to upstream:
> +https://github.com/shadow-maint/shadow/commit/e5905c4b84d4fb90aefcd96ee618411ebfac663d
> +
> +Signed-off-by: Xiangyu Chen <xiangyu.chen@windriver.com>
> +---
> + lib/fields.c | 11 +++++++----
> + 1 file changed, 7 insertions(+), 4 deletions(-)
> +
> +diff --git a/lib/fields.c b/lib/fields.c
> +index 640be931..fb51b582 100644
> +--- a/lib/fields.c
> ++++ b/lib/fields.c
> +@@ -21,9 +21,9 @@
> +  *
> +  * The supplied field is scanned for non-printable and other illegal
> +  * characters.
> +- *  + -1 is returned if an illegal character is present.
> +- *  +  1 is returned if no illegal characters are present, but the field
> +- *       contains a non-printable character.
> ++ *  + -1 is returned if an illegal or control character is present.
> ++ *  +  1 is returned if no illegal or control characters are present,
> ++ *       but the field contains a non-printable character.
> +  *  +  0 is returned otherwise.
> +  */
> + int valid_field (const char *field, const char *illegal)
> +@@ -45,10 +45,13 @@ int valid_field (const char *field, const char *illegal)
> +       }
> +
> +       if (0 == err) {
> +-              /* Search if there are some non-printable characters */
> ++              /* Search if there are non-printable or control characters */
> +               for (cp = field; '\0' != *cp; cp++) {
> +                       if (!isprint (*cp)) {
> +                               err = 1;
> ++                      }
> ++                      if (!iscntrl (*cp)) {
> ++                              err = -1;
> +                               break;
> +                       }
> +               }
> +--
> +2.34.1
> +
> diff --git a/meta/recipes-extended/shadow/shadow.inc b/meta/recipes-extended/shadow/shadow.inc
> index 5106b95571..3c1dd2f98e 100644
> --- a/meta/recipes-extended/shadow/shadow.inc
> +++ b/meta/recipes-extended/shadow/shadow.inc
> @@ -16,6 +16,8 @@ SRC_URI = "https://github.com/shadow-maint/shadow/releases/download/v${PV}/${BP}
>             ${@bb.utils.contains('PACKAGECONFIG', 'pam', '${PAM_SRC_URI}', '', d)} \
>             file://shadow-relaxed-usernames.patch \
>             file://useradd \
> +           file://CVE-2023-29383.patch \
> +           file://0001-Overhaul-valid_field.patch \
>             "
>
>  SRC_URI:append:class-target = " \
> --
> 2.34.1
>

Re: [kirkstone][PATCH v2] shadow: backport patch to fix CVE-2023-29383

[Pawan Badganchi]

[-- Attachment #1: Type: text/plain, Size: 244 bytes --]

Hi,
The issue does not seems to fix, getting below warning. Could you please check.

"configuration error - unknown item 'SYSLOG_SU_ENAB' (notify administrator)"
"configuration error - unknown item 'SYSLOG_SG_ENAB' (notify administrator)"

[-- Attachment #2: Type: text/html, Size: 406 bytes --]

Re: [kirkstone][PATCH v2] shadow: backport patch to fix CVE-2023-29383

[Pawan Badganchi]

[-- Attachment #1: Type: text/plain, Size: 596 bytes --]

Hi,
The issue does not seems to fix, getting below warning. Could you please check.

We are using this shadow library in our application.
When we compile our application we get below warning in log.do_prepare_recipe_sysroot

"configuration error - unknown item 'SYSLOG_SU_ENAB' (notify administrator)"
"configuration error - unknown item 'SYSLOG_SG_ENAB' (notify administrator)"

above warning is observed though below CVE is already available in our code kirkstone branch.

CVE-2023-29383 ( https://github.com/advisories/GHSA-p9w4-8hh8-crcx ).patch
0001-Overhaul-valid_field.patch.

[-- Attachment #2: Type: text/html, Size: 1699 bytes --]

Re: [OE-core] [kirkstone][PATCH v2] shadow: backport patch to fix CVE-2023-29383

[Martin Jansa]

[-- Attachment #1: Type: text/plain, Size: 1409 bytes --]

according to https://bugs.archlinux.org/task/69933
SYSLOG_SU_ENAB not supported when pam is enabled

Maybe you need to adjust:
oe-core/meta/recipes-extended/shadow/files/login.defs_shadow-sysroot:SYSLOG_SU_ENAB
            yes
?

On Fri, Feb 9, 2024 at 12:09 PM Pawan Badganchi <badganchipv@gmail.com>
wrote:

> Hi,
> The issue does not seems to fix, getting below warning. Could you please
> check.
>
> We are using this shadow library in our application.
> When we compile our application we get below warning in
> log.do_prepare_recipe_sysroot
>
> "configuration error - unknown item 'SYSLOG_SU_ENAB' (notify
> administrator)"
> "configuration error - unknown item 'SYSLOG_SG_ENAB' (notify
> administrator)"
>
> above warning is observed though below CVE is already available in our
> code kirkstone branch.
>
> CVE-2023-29383 <https://github.com/advisories/GHSA-p9w4-8hh8-crcx>.patch
> 0001-Overhaul-valid_field.patch.
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#195196):
> https://lists.openembedded.org/g/openembedded-core/message/195196
> Mute This Topic: https://lists.openembedded.org/mt/98361235/3617156
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [
> martin.jansa@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
>

[-- Attachment #2: Type: text/html, Size: 3307 bytes --]

Re: [kirkstone][PATCH v2] shadow: backport patch to fix CVE-2023-29383

[Pawan Badganchi]

[-- Attachment #1: Type: text/plain, Size: 348 bytes --]

Already set  oe-core/meta/recipes-extended/shadow/files/login.defs_shadow-sysroot:SYSLOG_SU_ENAB             yes
Still issue is coming.

# Enable "syslog" logging of su activity - in addition to sulog file logging.
# SYSLOG_SG_ENAB does the same for newgrp and sg.
#
SYSLOG_SU_ENAB          yes
SYSLOG_SG_ENAB          yes

[-- Attachment #2: Type: text/html, Size: 505 bytes --]

Re: [kirkstone][PATCH v2] shadow: backport patch to fix CVE-2023-29383

[Pawan Badganchi]

[-- Attachment #1: Type: text/plain, Size: 51 bytes --]

Hi,

Any idea furhter, what could be the issue?

[-- Attachment #2: Type: text/html, Size: 59 bytes --]

Re: [kirkstone][PATCH v2] shadow: backport patch to fix CVE-2023-29383

[Pawan Badganchi]

[-- Attachment #1: Type: text/plain, Size: 51 bytes --]

Hi,

Any idea furhter, what could be the issue?

[-- Attachment #2: Type: text/html, Size: 59 bytes --]

Re: [kirkstone][PATCH v2] shadow: backport patch to fix CVE-2023-29383

[Pawan Badganchi]

[-- Attachment #1: Type: text/plain, Size: 32 bytes --]

Hi,

Could please help here?

[-- Attachment #2: Type: text/html, Size: 40 bytes --]

Re: [OE-core] [kirkstone][PATCH v2] shadow: backport patch to fix CVE-2023-29383

[Fabio Berton]

[-- Attachment #1: Type: text/plain, Size: 1239 bytes --]

Hi,

I checked on kirkstone using OE-Core with DISTRO="nodistro" and when 
running bitbake useradd-example I can see this:

configuration error - unknown item 'SYSLOG_SU_ENAB' (notify administrator)
configuration error - unknown item 'SYSLOG_SG_ENAB' (notify administrator)

in the log.do_prepare_recipe_sysroot.

The same happens without CVE-2023-29383.patch and 
0001-Overhaul-valid_field.patch patches. I didn't understand why the 
SYSLOG_SU_ENAB is not supported.

What is the correct approach here, remove SYSLOG_SU_ENAB and 
SYSLOG_SG_ENAB from login.defs_shadow-sysroot?

To use the useradd-example.bb was needed to add this change 
https://lists.openembedded.org/g/openembedded-core/topic/kirkstone_patch/104757004

Thanks

On 2/26/2024 5:14 AM, Pawan Badganchi wrote:
> Hi,
>
> Could please help here?
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#196178):https://lists.openembedded.org/g/openembedded-core/message/196178
> Mute This Topic:https://lists.openembedded.org/mt/98361235/6083838
> Group Owner:openembedded-core+owner@lists.openembedded.org
> Unsubscribe:https://lists.openembedded.org/g/openembedded-core/unsub  [fbberton@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>

[-- Attachment #2: Type: text/html, Size: 2992 bytes --]

Re: [OE-core] [kirkstone][PATCH v2] shadow: backport patch to fix CVE-2023-29383

[Fabio Berton]

[-- Attachment #1: Type: text/plain, Size: 1557 bytes --]

Hi,


The error message is caused because the 
0001-Disable-use-of-syslog-for-sysroot.patch applied to the native 
recipe disables syslog support and the native tool does not recognize 
SYSLOG_SU_ENAB.

On 3/6/2024 12:13 AM, Fabio Berton via lists.openembedded.org wrote:
>
> Hi,
>
> I checked on kirkstone using OE-Core with DISTRO="nodistro" and when 
> running bitbake useradd-example I can see this:
>
> configuration error - unknown item 'SYSLOG_SU_ENAB' (notify administrator)
> configuration error - unknown item 'SYSLOG_SG_ENAB' (notify administrator)
>
> in the log.do_prepare_recipe_sysroot.
>
> The same happens without CVE-2023-29383.patch and 
> 0001-Overhaul-valid_field.patch patches. I didn't understand why the 
> SYSLOG_SU_ENAB is not supported.
>
> What is the correct approach here, remove SYSLOG_SU_ENAB and 
> SYSLOG_SG_ENAB from login.defs_shadow-sysroot?
>
> To use the useradd-example.bb was needed to add this change 
> https://lists.openembedded.org/g/openembedded-core/topic/kirkstone_patch/104757004
>
> Thanks
>
> On 2/26/2024 5:14 AM, Pawan Badganchi wrote:
>> Hi,
>>
>> Could please help here?
>>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#196655):https://lists.openembedded.org/g/openembedded-core/message/196655
> Mute This Topic:https://lists.openembedded.org/mt/98361235/6083838
> Group Owner:openembedded-core+owner@lists.openembedded.org
> Unsubscribe:https://lists.openembedded.org/g/openembedded-core/unsub  [fbberton@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>

[-- Attachment #2: Type: text/html, Size: 3868 bytes --]