| Message ID | 20230419061739.2924050-1-xiangyu.chen@eng.windriver.com |
|---|---|
| State | Accepted, archived |
| Commit | ab48ab23de6f6bb1f05689c97724140d4bef8faa |
| Headers | show |
| Series | [kirkstone,v2] shadow: backport patch to fix CVE-2023-29383 | expand |
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 >
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)"
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.
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]
> -=-=-=-=-=-=-=-=-=-=-=-
>
>
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
Hi, Any idea furhter, what could be the issue?
Hi, Any idea furhter, what could be the issue?
Hi, Could please help here?
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] > -=-=-=-=-=-=-=-=-=-=-=- >
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] > -=-=-=-=-=-=-=-=-=-=-=- >
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 = " \