On Wed, Jun 24, 2020 at 09:43:51PM -0500, Alexander Perlis wrote:
Although it seems that variable values themselves can be longer than 1022 bytes, if one places a dollar-variable-expansion inside another variable, then only the first 1022 bytes will be expanded. The internal buffer has size 1024, so why 1022 rather than 1023? The off-by-one error seems to be near the end of var_to_string(), where escape_string(val, len - 1, tmp) should likely be escape_string(val, len, tmp).
It looks like you're right. I'll take a look and commit a fix.
Looking at escape_string(), it incidentally seems there's an edge case where the escaping of a quotation mark at the end of the destination buffer would lead to not just truncation but also mutation (storing the incorrect last character). I believe the following patch would eliminate that possibility:
I wonder why they treated \ and " differently though: it appears purposely done. Let me take a closer look before changing anything; it's always the innocuous looking changes that come back to bite me.
As for increasing the variable expansion limit: if I follow the parsing code correctly, it mostly uses the BUFFER type, but parse_set() calls mutt_extract_token() which for variable expansion uses a local stack-based LONG_STRING (1024 bytes) to receive the value, and calls var_to_string(), which constructs the value inside another local stack-based LONG_STRING. Could these be changed to HUGE_STRING (8192 bytes)? This would allow for longer variable expansions, sufficient for what I was trying to do with my OAuth2 token parameter.
I've been working on removing HUGE_STRING stack usages, so although this is possible I'd rather just convert it to use the buffer pool. I'll try to do that today.
-- Kevin J. McCarthy GPG Fingerprint: 8975 A9B3 3AA3 7910 385C 5308 ADEF 7684 8031 6BDA
signature.asc
Description: PGP signature
