Re: [PATCH 1/2] iconvdata: SHIFT_JISX0213 lacks pending character reset (CVE-2026-77117)

On 8/26/26 6:35 AM, Florian Weimer wrote:
> * Carlos O'Donell:
> 
>> There is another bug present here that results in data loss and should
>> probably get it's own bug filed...
>>
>> What happens if the combining character is the last character?
>>
>> In such a case loop.c skips calling BODY (because there is no input to
>> consume) in that case and returns a success but drops the combining
>> character even if there is room, and this leads to a data loss scenario?

Let me reiterate that this is orthogonal to the CVE and should not
block you pushing fixes for the two CVEs which are converter hangs. 
> As implemented, SHIFT_JISX0213 has shift states due to this pending
> character handling.  This means that applications need to flush the
> state to drain any pending characters.  I don't think there is a bug.
I think it is a bug, and let me expand why.

(1) POSIX rules.

Your interpretation requires the application to both enlarge the
buffer, and then flush after any E2BIG errors.

(1.a) The flush is not required.

My interpretation of POSIX is that it does not require flushing.

https://pubs.opengroup.org/onlinepubs/9799919799/functions/iconv.html

"Subsequent calls with inbuf as other than a null pointer or a pointer
  to a null pointer cause the conversion to take place from the current
  state of the conversion descriptor."

This bug is only observable if you run out of space after consuming
input, recording it to *statep, and this only happens if you have
a combining character that can't be written due to lack of space
*and* have no subsequent input to trigger a BODY loop.

I expect this has caused real and obscure bugs in the field.

Asking an LLM, which is what a new developer might do today, says:

"You can continue directly from the current state without flushing or
  resetting the converter. An E2BIG error simply means the output
  buffer ran out of space; it does not corrupt the conversion
  descriptor (iconv_t) or break stateful encodings."

(1.b) Performance.

In order to minimize the need to flush you'll have to check for
complete consumption of input bytes and then flush. This isn't required
by POSIX, so why should glibc require it?

> At the iconv level, where inputs and outputs are opaque byte streams,
> SHIFT_JISX0213 could be implemented with shift states.  But this would
> preclude use of SHIFT_JISX0213 as a locale character set because the C
> interfaces assume that one input character can produce at most one
> output character, and glibc (and other implementations) use shift states
> to work around that to some extent.
I don't follow why it would preclude it.

I also don't understand how shift states would work around this.

Do you have examples?

The converter consumes 1 character, and writes 2 output characters,
but finds it can only write 1 output and returns E2BIG. The second combining
character is temporarily stored (as-if a 1 character buffer is used).

Why isn't the fix of the following form?

diff --git a/iconvdata/shift_jisx0213.c b/iconvdata/shift_jisx0213.c
index e9179f605e..1539355078 100644
--- a/iconvdata/shift_jisx0213.c
+++ b/iconvdata/shift_jisx0213.c
@@ -193,32 +193,29 @@
                 STANDARD_FROM_LOOP_ERR_HANDLER (1);                           \
               }                                                               \
                                                                               \
-           inptr += 2;                                                       \
-                                                                             \
             if (ch < 0x80)                                                    \
               {                                                               \
                 /* It's a combining character.  */                            \
                 uint32_t u1 = __jisx0213_to_ucs_combining[ch - 1][0];         \
                 uint32_t u2 = __jisx0213_to_ucs_combining[ch - 1][1];         \
                                                                               \
-               put32 (outptr, u1);                                           \
-               outptr += 4;                                                  \
-                                                                             \
                 /* See whether we have room for two characters.  */           \
                 if (outptr + 4 <= outend)                                     \
                   {                                                           \
+                   inptr += 2;                                               \
+                   put32 (outptr, u1);                                       \
+                   outptr += 4;                                              \
                     put32 (outptr, u2);                                       \
                     outptr += 4;                                              \
                     continue;                                                 \
                   }                                                           \
                                                                               \
-               /* Otherwise store only the first character now, and          \
-                  put the second one into the queue.  */                     \
-               *statep = u2 << 3;                                            \
                 /* Tell the caller why we terminate the loop.  */             \
                 result = __GCONV_FULL_OUTPUT;                                 \
                 break;                                                        \
               }                                                               \
+                                                                             \
+           inptr += 2;                                                       \
           }                                                                   \
         else                                                                  \
           {                                                                   \
~~~

This is obviously not a full cleanup, but in the SHIFT_JISX0213 to UCS-4
we no longer need the use of the state variable.

I think this is a logic bug in the converter, we should have looked to
see if we could write out the two characters to the output buffer.

A partial output could also work, but that would require reworking the
iconv/loop.c code to process intermediate converter state too, and that's a
more invasive change.

> So I think there is no data loss bug here.
Does my argument above change your position?

-- 
Cheers,
Carlos.