[PATCH] s6-unquote*: it's memchr(str, char, len), not memchr(str, len, char)

From: Wictor Lund <wlund_at_iki.fi>
Date: Wed, 10 Jun 2020 16:26:22 +0300

Hi,

I observed that there's was somethings strange going on with
s6-unquote-filter. On OpenBSD/amd64 I get

    $ s6-quote str | s6-unquote-filter
    s6-unquote-filter: warning: invalid starting quote character
    "str"
    $

So I checked on Alpinelinux armhf architecture

    $ s6-quote str | s6-unquote-filter
    s6-unquote-filter: warning: invalid starting quote character
    "str"
    $

(the binaries are from the Alpinelinux armhf package
s6-portable-utils-2.2.2.1-r0)

Ok, so then I checked Alpinelinux aarch64 architeture

    $ s6-quote str | s6-unquote-filter
    str
    $

(the binaries are from the Alpinelinux aarch64 package
s6-portable-utils-2.2.2.1-r0)

Also I found that for instance, astr" is accepted on aarch64 as an quoted
input string to s6-unquote-filter, while on the "buggy" machines astr" is
accepted to the same extent as "str".

When I found the cause of this, I got even more confused, because how can
this code run at all in the first place and why doesn't it cause a memory
page violation when trying to find the character (int)delimlen in a string
of size *s?


The OpenBSD binary looks like this around the "\"" string:
00000950 75 6e 71 75 6f 74 65 2d 66 69 6c 74 65 72 00 3a |unquote-filter.:|
00000960 20 77 61 72 6e 69 6e 67 3a 20 00 71 51 76 77 64 | warning: .qQvwd|
00000970 3a 00 63 61 6e 27 74 20 68 61 70 70 65 6e 3a 20 |:.can't happen: |
00000980 75 6e 6b 6e 6f 77 6e 20 73 74 72 69 63 74 6e 65 |unknown strictne|
00000990 73 73 00 22 00 77 72 69 74 65 20 74 6f 20 73 74 |ss.".write to st|
000009a0 64 6f 75 74 00 3a 20 75 73 61 67 65 3a 20 00 65 |dout.: usage: .e|
000009b0 6d 70 74 79 20 6c 69 6e 65 00 20 69 6e 20 6c 69 |mpty line. in li|
000009c0 6e 65 3a 20 00 2c 20 69 67 6e 6f 72 69 6e 67 20 |ne: ., ignoring |

This is for one version of the binary, it seems that the string allocations
are permutated in some kind of deterministic random fashion. I haven't
checked the details.

The alpinelinux armhf binary looks like this around the "\"" string:
00001660 75 6f 74 65 20 63 68 61 72 61 63 74 65 72 20 61 |uote character a|
00001670 74 20 70 6f 73 69 74 69 6f 6e 20 00 2c 20 69 67 |t position ., ig|
00001680 6e 6f 72 69 6e 67 20 72 65 6d 61 69 6e 64 65 72 |noring remainder|
00001690 20 6f 66 20 00 2f 00 77 72 69 74 65 20 74 6f 20 | of ./.write to |
000016a0 73 74 64 6f 75 74 00 0a 00 22 00 00 00 00 00 00 |stdout..."......|
000016b0 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
*
00001e80 00 00 00 00 00 00 00 00 0c 14 00 00 80 13 00 00 |................|
00001e90 01 00 00 00 01 00 00 00 01 00 00 00 60 01 00 00 |............`...|
00001ea0 0c 00 00 00 5c 06 00 00 0d 00 00 00 20 15 00 00 |....\....... ...|
00001eb0 19 00 00 00 88 1e 01 00 1b 00 00 00 04 00 00 00 |................|
00001ec0 1a 00 00 00 8c 1e 01 00 1c 00 00 00 04 00 00 00 |................|

We see that read zeros are far the eye's can see, i.e. '\"' = 34 bytes.

The alpinelinux aarch64 binary looks like this around the "\"" string:
00001630 63 74 65 72 00 20 69 6e 20 00 2c 20 69 67 6e 6f |cter. in ., igno|
00001640 72 69 6e 67 20 72 65 6d 61 69 6e 64 65 72 20 6f |ring remainder o|
00001650 66 20 00 2f 00 66 6f 75 6e 64 20 65 6e 64 69 6e |f ./.found endin|
00001660 67 20 71 75 6f 74 65 20 63 68 61 72 61 63 74 65 |g quote characte|
00001670 72 20 61 74 20 70 6f 73 69 74 69 6f 6e 20 00 77 |r at position .w|
00001680 72 69 74 65 20 74 6f 20 73 74 64 6f 75 74 00 0a |rite to stdout..|
00001690 00 22 00 00 01 1b 03 3b 28 00 00 00 04 00 00 00 |.".....;(.......|
000016a0 8c fc ff ff 40 00 00 00 bc fc ff ff 54 00 00 00 |...._at_.......T...|
000016b0 f8 fc ff ff 68 00 00 00 60 fd ff ff 8c 00 00 00 |....h...`.......|
000016c0 10 00 00 00 00 00 00 00 01 7a 52 00 04 78 1e 01 |.........zR..x..|
000016d0 1b 0c 1f 00 10 00 00 00 18 00 00 00 44 fc ff ff |............D...|
000016e0 30 00 00 00 00 00 00 00 10 00 00 00 2c 00 00 00 |0...........,...|

Here we find a '\x1' almost immediately.

--
Wictor Lund
---
 src/skaembutils/s6-unquote-filter.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/skaembutils/s6-unquote-filter.c b/src/skaembutils/s6-unquote-filter.c
index edf2682..47a085c 100644
--- a/src/skaembutils/s6-unquote-filter.c
+++ b/src/skaembutils/s6-unquote-filter.c
_at_@ -46,7 +46,7 @@ static int doit (char const *s, size_t len)
       }
       return 1 ;
     }
-    if (!memchr(delim, delimlen, *s))
+    if (!memchr(delim, *s, delimlen))
     {
       switch (strictness)
       {
-- 
2.26.2
Received on Wed Jun 10 2020 - 13:26:22 UTC

This archive was generated by hypermail 2.3.0 : Sun May 09 2021 - 19:38:49 UTC