Skip to content

Base64 Content-Transfer-Encoding parts are decoded per-chunk, corrupting stored data #280

Closed

Description

@UlisesGascon

Summary

Form.prototype.onParsePartData decodes each network chunk of a Content-Transfer-Encoding: base64 part as an independent base64 string, with no decoder state carried between chunks. Node's base64 decoder drops the 1 to 3 trailing characters that do not complete a 4-character group, and because no remainder is kept, those characters are lost. As a result the bytes multiparty writes to disk depend on where the incoming data was split into chunks rather than on the content of the request, so the same upload can be stored differently depending on network chunking or an upstream proxy's buffering.

The project already avoids this on the header and field paths by using a stateful string_decoder (headerFieldDecoder / headerValueDecoder, and handleField). The base64 part path is the one place a stateful decoder is needed and not used, so this is a self-contained fix that mirrors code already in the file. Good for a first contribution.

Affected code

index.js (unchanged since the streams2 rewrite in f19bc82 and released on 2.1.0:

Form.prototype.onParsePartData = function(b) {
  if (this.partTransferEncoding === 'base64') {
    this.backpressure = ! this.destStream.write(b.toString('ascii'), 'base64');
  } else {
    this.backpressure = ! this.destStream.write(b);
  }
}

b is a slice of a single network chunk, so each call runs a fresh Buffer.from(str, 'base64') and any incomplete trailing quantum is discarded. b.toString('ascii') also masks the high bit, so a byte >= 0x80 becomes a different valid base64 character instead of being rejected.

How to reproduce

Send a Content-Transfer-Encoding: base64 part and split the writes so the base64 body does not arrive on 4-character boundaries. Two controls isolate the cause:

  • The same file sent base64-encoded in a single write is stored correctly.
  • The same file sent as a binary part split across several writes is stored correctly.
  • Only base64 and chunking together corrupt the stored file, on every attempt (not a race).

Suggested approach

this is a quite brainstorm and might not be the final implementation (just some ideas)

  1. In clearPartVars, initialise a remainder buffer: self.partB64Remainder = ''.
  2. In onParsePartData, prepend the remainder, decode only the complete 4-character quantums, and keep the leftover for the next chunk. Use latin1 instead of ascii so high-bit bytes are not turned into a different valid base64 character:
Form.prototype.onParsePartData = function(b) {
  if (this.partTransferEncoding === 'base64') {
    var s = this.partB64Remainder + b.toString('latin1');
    var cut = s.length - (s.length % 4);
    this.partB64Remainder = s.slice(cut);
    this.backpressure = ! this.destStream.write(Buffer.from(s.slice(0, cut), 'base64'));
  } else {
    this.backpressure = ! this.destStream.write(b);
  }
}
  1. In onParsePartEnd, flush partB64Remainder before s.end(), rejecting a remainder of length 1 (never valid base64) rather than discarding it.

How to verify

Add a test that uploads a base64 part whose body is written across chunk boundaries that do not align to 4 characters, and assert the stored file equals the original bytes. A matching control that sends the same body in a single write should already pass. Run npm test and npm run lint.

Acceptance criteria

  • A base64 part split across arbitrary chunk boundaries decodes to the same bytes as the single-write case.
  • A regression test covers the split-chunk case.
  • Existing tests and lint pass.

Reported and initial analysis done by @euriconicacio (internal ref).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions