Re: ober_scanf_elements() and empty sequences
On Tue, 2023-08-22 at 13:27 +0200, Martijn van Duren wrote:
> On Tue, 2023-08-22 at 10:16 +, Gerhard Roth wrote:
> > On Tue, 2023-08-22 at 11:16 +0200, Martijn van Duren wrote:
> > > On Mon, 2023-08-21 at 07:35 +, Gerhard Roth wrote:
> > > > Hi Martijn,
> > > >
> > > > last November you fixed ber.c so that sequences won't generate
> > > > an uninitialized subelement.
> > > >
> > > > This revealed another bug in ober_scanf_elements(): it couldn't
> > > > process sequences with an empty list of subelements. The following
> > > > code failed in ober_scanf_elements():
> > > >
> > > > struct ber_element *root;
> > > > struct ber_element *sub;
> > > >
> > > > if ((root = ober_add_sequence(NULL)) == NULL)
> > > > err(1, "ober_add_sequence() failed");
> > > >
> > > > errno = 0;
> > > > if (ober_scanf_elements(root, "{e", ))
> > > > err(1, "ober_scanf_elements() failed");
> > > >
> > > > printf("sub = %p\n", sub);
> > > >
> > > >
> > > > The patch below fixes that.
> > > >
> > > > Gerhard
> > > >
> > > >
> > > > Index: lib/libutil/ber.c
> > > > ===
> > > > RCS file: /cvs/src/lib/libutil/ber.c,v
> > > > retrieving revision 1.24
> > > > diff -u -p -u -p -r1.24 ber.c
> > > > --- lib/libutil/ber.c 3 Nov 2022 17:58:10 - 1.24
> > > > +++ lib/libutil/ber.c 21 Aug 2023 07:24:21 -
> > > > @@ -700,7 +700,8 @@ ober_scanf_elements(struct ber_element *
> > > >
> > > > va_start(ap, fmt);
> > > > while (*fmt) {
> > > > - if (ber == NULL && *fmt != '$' && *fmt != '}' && *fmt
> > > > != ')')
> > > > + if (ber == NULL && *fmt != '$' && *fmt != '}' && *fmt
> > > > != ')' &&
> > > > + *fmt != 'e')
> > > > goto fail;
> > >
> > > I'm not sure about this part. An ober_scanf_elements of "{}e" on your
> > > example above also fails. The 'e' element might not increment the ber
> > > pointer, but I do think it should fail if an expected element is
> > > missing.
> >
> > You're right. And digging into it, only makes it look worse.
> >
> > I think, the 'e' format should behave differently.
> >
> > 1) in case of '{e' it is the first element of a sequence.
> > Empty sequences are allowed and we should not fail but
> > set the argument to NULL instead.
> >
> > 2) in all other cases, 'e' is the next element in the list.
> > And here we should fail if there are less elements in the
> > list than expected.
> >
> > Below is an updated diff that fixes the problem by remembering
> > whether the last traversal was via 'be_sub' (fsub == 1) or 'be_next'
> > and then behaves differently. Not very elegant although.
>
> I'm not sure yet. Do you have a particular usecase for this?
> Usually you would just do something like
> ober_scanf_elements(elm, "e{", seq);
> if (seq->be_sub != NULL) ...
In my use-case the format contained more elements that followed.
I wanted to scan them all with a single ober_scanf_elements().
But I can just as well split that up into two separate call.
So let's forget about this patch.
Many thanks for looking into this!
Gerhard
>
> >
> > >
> > > > switch (*fmt++) {
> > > > case '$':
> > > > @@ -797,7 +798,7 @@ ober_scanf_elements(struct ber_element *
> > > > if (ber->be_encoding != BER_TYPE_SEQUENCE &&
> > > > ber->be_encoding != BER_TYPE_SET)
> > > > goto fail;
> > > > - if (ber->be_sub == NULL || level >= _MAX_SEQ-1)
> > > > + if (level >= _MAX_SEQ-1)
> > >
> > > This part is OK martijn@
> > >
> > > > goto fail;
> > > > parent[++level] = ber;
> > > > ber = ber->be_sub;
> > > >
> > >
> >
> > Index: lib/libutil/ber.c
> > ===
> > RCS file: /cvs/src/lib/libutil/ber.c,v
> > retrieving revision 1.24
> > diff -u -p -u -p -r1.24 ber.c
> > --- lib/libutil/ber.c 3 Nov 2022 17:58:10 - 1.24
> > +++ lib/libutil/ber.c 22 Aug 2023 10:10:43 -
> > @@ -695,12 +695,14 @@ ober_scanf_elements(struct ber_element *
> > off_t *pos;
> > struct ber_oid *o;
> > struct ber_element *parent[_MAX_SEQ], **e;
> > + int fsub = 0;
> >
> > memset(parent, 0, sizeof(struct ber_element *) * _MAX_SEQ);
> >
> > va_start(ap, fmt);
> > while (*fmt) {
> > - if (ber == NULL && *fmt != '$' && *fmt != '}' && *fmt !=
> > ')')
> > + if (ber == NULL && *fmt != '$' && *fmt != '}' && *fmt !=
> > ')' &&
> > + *fmt != 'e')
> > goto fail;
> >
Re: ober_scanf_elements() and empty sequences
On Tue, 2023-08-22 at 10:16 +, Gerhard Roth wrote:
> On Tue, 2023-08-22 at 11:16 +0200, Martijn van Duren wrote:
> > On Mon, 2023-08-21 at 07:35 +, Gerhard Roth wrote:
> > > Hi Martijn,
> > >
> > > last November you fixed ber.c so that sequences won't generate
> > > an uninitialized subelement.
> > >
> > > This revealed another bug in ober_scanf_elements(): it couldn't
> > > process sequences with an empty list of subelements. The following
> > > code failed in ober_scanf_elements():
> > >
> > > struct ber_element *root;
> > > struct ber_element *sub;
> > >
> > > if ((root = ober_add_sequence(NULL)) == NULL)
> > > err(1, "ober_add_sequence() failed");
> > >
> > > errno = 0;
> > > if (ober_scanf_elements(root, "{e", ))
> > > err(1, "ober_scanf_elements() failed");
> > >
> > > printf("sub = %p\n", sub);
> > >
> > >
> > > The patch below fixes that.
> > >
> > > Gerhard
> > >
> > >
> > > Index: lib/libutil/ber.c
> > > ===
> > > RCS file: /cvs/src/lib/libutil/ber.c,v
> > > retrieving revision 1.24
> > > diff -u -p -u -p -r1.24 ber.c
> > > --- lib/libutil/ber.c 3 Nov 2022 17:58:10 - 1.24
> > > +++ lib/libutil/ber.c 21 Aug 2023 07:24:21 -
> > > @@ -700,7 +700,8 @@ ober_scanf_elements(struct ber_element *
> > >
> > > va_start(ap, fmt);
> > > while (*fmt) {
> > > - if (ber == NULL && *fmt != '$' && *fmt != '}' && *fmt !=
> > > ')')
> > > + if (ber == NULL && *fmt != '$' && *fmt != '}' && *fmt !=
> > > ')' &&
> > > + *fmt != 'e')
> > > goto fail;
> >
> > I'm not sure about this part. An ober_scanf_elements of "{}e" on your
> > example above also fails. The 'e' element might not increment the ber
> > pointer, but I do think it should fail if an expected element is
> > missing.
>
> You're right. And digging into it, only makes it look worse.
>
> I think, the 'e' format should behave differently.
>
> 1) in case of '{e' it is the first element of a sequence.
> Empty sequences are allowed and we should not fail but
> set the argument to NULL instead.
>
> 2) in all other cases, 'e' is the next element in the list.
> And here we should fail if there are less elements in the
> list than expected.
>
> Below is an updated diff that fixes the problem by remembering
> whether the last traversal was via 'be_sub' (fsub == 1) or 'be_next'
> and then behaves differently. Not very elegant although.
I'm not sure yet. Do you have a particular usecase for this?
Usually you would just do something like
ober_scanf_elements(elm, "e{", seq);
if (seq->be_sub != NULL) ...
>
> >
> > > switch (*fmt++) {
> > > case '$':
> > > @@ -797,7 +798,7 @@ ober_scanf_elements(struct ber_element *
> > > if (ber->be_encoding != BER_TYPE_SEQUENCE &&
> > > ber->be_encoding != BER_TYPE_SET)
> > > goto fail;
> > > - if (ber->be_sub == NULL || level >= _MAX_SEQ-1)
> > > + if (level >= _MAX_SEQ-1)
> >
> > This part is OK martijn@
> >
> > > goto fail;
> > > parent[++level] = ber;
> > > ber = ber->be_sub;
> > >
> >
>
> Index: lib/libutil/ber.c
> ===
> RCS file: /cvs/src/lib/libutil/ber.c,v
> retrieving revision 1.24
> diff -u -p -u -p -r1.24 ber.c
> --- lib/libutil/ber.c 3 Nov 2022 17:58:10 - 1.24
> +++ lib/libutil/ber.c 22 Aug 2023 10:10:43 -
> @@ -695,12 +695,14 @@ ober_scanf_elements(struct ber_element *
> off_t *pos;
> struct ber_oid *o;
> struct ber_element *parent[_MAX_SEQ], **e;
> + int fsub = 0;
>
> memset(parent, 0, sizeof(struct ber_element *) * _MAX_SEQ);
>
> va_start(ap, fmt);
> while (*fmt) {
> - if (ber == NULL && *fmt != '$' && *fmt != '}' && *fmt != ')')
> + if (ber == NULL && *fmt != '$' && *fmt != '}' && *fmt != ')' &&
> + *fmt != 'e')
> goto fail;
> switch (*fmt++) {
> case '$':
> @@ -732,6 +734,8 @@ ober_scanf_elements(struct ber_element *
> case 'e':
> e = va_arg(ap, struct ber_element **);
> *e = ber;
> + if (fsub)
> + goto fail;
> ret++;
> continue;
> case 'E':
> @@ -797,10 +801,11 @@ ober_scanf_elements(struct ber_element *
> if (ber->be_encoding != BER_TYPE_SEQUENCE &&
> ber->be_encoding != BER_TYPE_SET)
>
Re: ober_scanf_elements() and empty sequences
On Tue, 2023-08-22 at 11:16 +0200, Martijn van Duren wrote:
> On Mon, 2023-08-21 at 07:35 +, Gerhard Roth wrote:
> > Hi Martijn,
> >
> > last November you fixed ber.c so that sequences won't generate
> > an uninitialized subelement.
> >
> > This revealed another bug in ober_scanf_elements(): it couldn't
> > process sequences with an empty list of subelements. The following
> > code failed in ober_scanf_elements():
> >
> > struct ber_element *root;
> > struct ber_element *sub;
> >
> > if ((root = ober_add_sequence(NULL)) == NULL)
> > err(1, "ober_add_sequence() failed");
> >
> > errno = 0;
> > if (ober_scanf_elements(root, "{e", ))
> > err(1, "ober_scanf_elements() failed");
> >
> > printf("sub = %p\n", sub);
> >
> >
> > The patch below fixes that.
> >
> > Gerhard
> >
> >
> > Index: lib/libutil/ber.c
> > ===
> > RCS file: /cvs/src/lib/libutil/ber.c,v
> > retrieving revision 1.24
> > diff -u -p -u -p -r1.24 ber.c
> > --- lib/libutil/ber.c 3 Nov 2022 17:58:10 - 1.24
> > +++ lib/libutil/ber.c 21 Aug 2023 07:24:21 -
> > @@ -700,7 +700,8 @@ ober_scanf_elements(struct ber_element *
> >
> > va_start(ap, fmt);
> > while (*fmt) {
> > - if (ber == NULL && *fmt != '$' && *fmt != '}' && *fmt !=
> > ')')
> > + if (ber == NULL && *fmt != '$' && *fmt != '}' && *fmt !=
> > ')' &&
> > + *fmt != 'e')
> > goto fail;
>
> I'm not sure about this part. An ober_scanf_elements of "{}e" on your
> example above also fails. The 'e' element might not increment the ber
> pointer, but I do think it should fail if an expected element is
> missing.
You're right. And digging into it, only makes it look worse.
I think, the 'e' format should behave differently.
1) in case of '{e' it is the first element of a sequence.
Empty sequences are allowed and we should not fail but
set the argument to NULL instead.
2) in all other cases, 'e' is the next element in the list.
And here we should fail if there are less elements in the
list than expected.
Below is an updated diff that fixes the problem by remembering
whether the last traversal was via 'be_sub' (fsub == 1) or 'be_next'
and then behaves differently. Not very elegant although.
>
> > switch (*fmt++) {
> > case '$':
> > @@ -797,7 +798,7 @@ ober_scanf_elements(struct ber_element *
> > if (ber->be_encoding != BER_TYPE_SEQUENCE &&
> > ber->be_encoding != BER_TYPE_SET)
> > goto fail;
> > - if (ber->be_sub == NULL || level >= _MAX_SEQ-1)
> > + if (level >= _MAX_SEQ-1)
>
> This part is OK martijn@
>
> > goto fail;
> > parent[++level] = ber;
> > ber = ber->be_sub;
> >
>
Index: lib/libutil/ber.c
===
RCS file: /cvs/src/lib/libutil/ber.c,v
retrieving revision 1.24
diff -u -p -u -p -r1.24 ber.c
--- lib/libutil/ber.c 3 Nov 2022 17:58:10 - 1.24
+++ lib/libutil/ber.c 22 Aug 2023 10:10:43 -
@@ -695,12 +695,14 @@ ober_scanf_elements(struct ber_element *
off_t *pos;
struct ber_oid *o;
struct ber_element *parent[_MAX_SEQ], **e;
+ int fsub = 0;
memset(parent, 0, sizeof(struct ber_element *) * _MAX_SEQ);
va_start(ap, fmt);
while (*fmt) {
- if (ber == NULL && *fmt != '$' && *fmt != '}' && *fmt != ')')
+ if (ber == NULL && *fmt != '$' && *fmt != '}' && *fmt != ')' &&
+ *fmt != 'e')
goto fail;
switch (*fmt++) {
case '$':
@@ -732,6 +734,8 @@ ober_scanf_elements(struct ber_element *
case 'e':
e = va_arg(ap, struct ber_element **);
*e = ber;
+ if (fsub)
+ goto fail;
ret++;
continue;
case 'E':
@@ -797,10 +801,11 @@ ober_scanf_elements(struct ber_element *
if (ber->be_encoding != BER_TYPE_SEQUENCE &&
ber->be_encoding != BER_TYPE_SET)
goto fail;
- if (ber->be_sub == NULL || level >= _MAX_SEQ-1)
+ if (level >= _MAX_SEQ-1)
goto fail;
parent[++level] = ber;
ber = ber->be_sub;
+ fsub = 1;
ret++;
continue;
case '}':
@@
Re: ober_scanf_elements() and empty sequences
On Mon, 2023-08-21 at 07:35 +, Gerhard Roth wrote:
> Hi Martijn,
>
> last November you fixed ber.c so that sequences won't generate
> an uninitialized subelement.
>
> This revealed another bug in ober_scanf_elements(): it couldn't
> process sequences with an empty list of subelements. The following
> code failed in ober_scanf_elements():
>
> struct ber_element *root;
> struct ber_element *sub;
>
> if ((root = ober_add_sequence(NULL)) == NULL)
> err(1, "ober_add_sequence() failed");
>
> errno = 0;
> if (ober_scanf_elements(root, "{e", ))
> err(1, "ober_scanf_elements() failed");
>
> printf("sub = %p\n", sub);
>
>
> The patch below fixes that.
>
> Gerhard
>
>
> Index: lib/libutil/ber.c
> ===
> RCS file: /cvs/src/lib/libutil/ber.c,v
> retrieving revision 1.24
> diff -u -p -u -p -r1.24 ber.c
> --- lib/libutil/ber.c 3 Nov 2022 17:58:10 - 1.24
> +++ lib/libutil/ber.c 21 Aug 2023 07:24:21 -
> @@ -700,7 +700,8 @@ ober_scanf_elements(struct ber_element *
>
> va_start(ap, fmt);
> while (*fmt) {
> - if (ber == NULL && *fmt != '$' && *fmt != '}' && *fmt != ')')
> + if (ber == NULL && *fmt != '$' && *fmt != '}' && *fmt != ')' &&
> + *fmt != 'e')
> goto fail;
I'm not sure about this part. An ober_scanf_elements of "{}e" on your
example above also fails. The 'e' element might not increment the ber
pointer, but I do think it should fail if an expected element is
missing.
> switch (*fmt++) {
> case '$':
> @@ -797,7 +798,7 @@ ober_scanf_elements(struct ber_element *
> if (ber->be_encoding != BER_TYPE_SEQUENCE &&
> ber->be_encoding != BER_TYPE_SET)
> goto fail;
> - if (ber->be_sub == NULL || level >= _MAX_SEQ-1)
> + if (level >= _MAX_SEQ-1)
This part is OK martijn@
> goto fail;
> parent[++level] = ber;
> ber = ber->be_sub;
>
ober_scanf_elements() and empty sequences
Hi Martijn,
last November you fixed ber.c so that sequences won't generate
an uninitialized subelement.
This revealed another bug in ober_scanf_elements(): it couldn't
process sequences with an empty list of subelements. The following
code failed in ober_scanf_elements():
struct ber_element *root;
struct ber_element *sub;
if ((root = ober_add_sequence(NULL)) == NULL)
err(1, "ober_add_sequence() failed");
errno = 0;
if (ober_scanf_elements(root, "{e", ))
err(1, "ober_scanf_elements() failed");
printf("sub = %p\n", sub);
The patch below fixes that.
Gerhard
Index: lib/libutil/ber.c
===
RCS file: /cvs/src/lib/libutil/ber.c,v
retrieving revision 1.24
diff -u -p -u -p -r1.24 ber.c
--- lib/libutil/ber.c 3 Nov 2022 17:58:10 - 1.24
+++ lib/libutil/ber.c 21 Aug 2023 07:24:21 -
@@ -700,7 +700,8 @@ ober_scanf_elements(struct ber_element *
va_start(ap, fmt);
while (*fmt) {
- if (ber == NULL && *fmt != '$' && *fmt != '}' && *fmt != ')')
+ if (ber == NULL && *fmt != '$' && *fmt != '}' && *fmt != ')' &&
+ *fmt != 'e')
goto fail;
switch (*fmt++) {
case '$':
@@ -797,7 +798,7 @@ ober_scanf_elements(struct ber_element *
if (ber->be_encoding != BER_TYPE_SEQUENCE &&
ber->be_encoding != BER_TYPE_SET)
goto fail;
- if (ber->be_sub == NULL || level >= _MAX_SEQ-1)
+ if (level >= _MAX_SEQ-1)
goto fail;
parent[++level] = ber;
ber = ber->be_sub;
smime.p7s
Description: S/MIME cryptographic signature
