threads / patch / 46817

patchmailinfo: don't decode invalid =XY quoted-printable sequences

Subject: [PATCH] mailinfo: don't decode invalid =XY quoted-printable sequences

## tl;dr

3 messages between Sep 23, 2017 and Sep 25, 2017. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

René Scharfe· Sep 23, 2017, 18:04 UTC · lore

Decode =XY in quoted-printable segments only if X and Y are hexadecimal digits, otherwise just copy them. That's at least better than interpreting negative results from hexval() as a character.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
 mailinfo.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

Show changes to mailinfo.c +8 −3

diff --git a/mailinfo.c b/mailinfo.c
index f2387a3267..3d8ac07399 100644
--- a/mailinfo.c
+++ b/mailinfo.c
@@ -367,11 +367,16 @@ static struct strbuf *decode_q_segment(const struct strbuf *q_seg, int rfc2047)
 
 	while ((c = *in++) != 0) {
 		if (c == '=') {
-			int d = *in++;
+			int ch, d = *in;
 			if (d == '\n' || !d)
 				break; /* drop trailing newline */
-			strbuf_addch(out, (hexval(d) << 4) | hexval(*in++));
-			continue;
+			ch = hex2chr(in);
+			if (ch >= 0) {
+				strbuf_addch(out, ch);
+				in += 2;
+				continue;
+			}
+			/* garbage -- fall through */
 		}
 		if (rfc2047 && c == '_') /* rfc2047 4.2 (2) */
 			c = 0x20;

-- 
2.14.1

René Scharfe· Sep 23, 2017, 18:36 UTC · re: René Scharfe · lore

Re: [PATCH] mailinfo: don't decode invalid =XY quoted-printable sequences

Am 23.09.2017 um 20:04 schrieb René Scharfe:

> Decode =XY in quoted-printable segments only if X and Y are hexadecimal
> digits, otherwise just copy them.  That's at least better than
> interpreting negative results from hexval() as a character.

Forgot to add:

Reported-by: Jeff King <peff@peff.net>

René

Jeff King· Sep 25, 2017, 12:28 UTC · re: René Scharfe · lore

Re: [PATCH] mailinfo: don't decode invalid =XY quoted-printable sequences

On Sat, Sep 23, 2017 at 08:04:40PM +0200, René Scharfe wrote:

> Decode =XY in quoted-printable segments only if X and Y are hexadecimal
> digits, otherwise just copy them.  That's at least better than
> interpreting negative results from hexval() as a character.

Thanks, this looks good to me overall.

I wondered if we should die() here, but walking over cruft may be more friendly. The base64 case does the same, though it actually ignores the bytes rather than copying them. Since this is never _supposed_ to happen, it's hard to say what behavior would be preferable without seeing a real-world broken case.

-Peff