Skip to content

Commit b73d8fa

Browse files
martinknafvemartinknafve
authored
Fix: Long ZIP filenames and foreign " ö "cause 'block attachment' renaming and conversion errors #145 (#516)
1 parent 5e46b81 commit b73d8fa

4 files changed

Lines changed: 419 additions & 23 deletions

File tree

‎hmailserver/source/Server/Common/Mime/Mime.cpp‎

Lines changed: 92 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,27 @@ namespace HM
145145

146146
bool encodedParameter = false;
147147

148-
std::vector<AnsiString> parameters = StringParser::SplitString(AnsiString(value_), ";");
148+
// Split on ';' while respecting quoted strings, so that parameter values
149+
// which legally contain semicolons (e.g. filename="semi;colon.dll") are
150+
// not broken across segments.
151+
std::vector<AnsiString> parameters;
152+
{
153+
AnsiString current;
154+
bool inQuote = false;
155+
for (size_t i = 0; i < value_.size(); i++)
156+
{
157+
char c = value_[i];
158+
if (c == '"') inQuote = !inQuote;
159+
if (!inQuote && c == ';')
160+
{
161+
parameters.push_back(current);
162+
current = "";
163+
}
164+
else
165+
current += c;
166+
}
167+
parameters.push_back(current);
168+
}
149169

150170
for (unsigned int i = 1; i < parameters.size(); i++)
151171
{
@@ -193,39 +213,47 @@ namespace HM
193213
valuePos++;
194214

195215
// Locate the start of the actual value. May be enclosed with quotes.
196-
//
197-
// For instance, this is perfectly valid
216+
// Track whether it is quoted so we can find the correct closing delimiter.
217+
//
218+
// For instance, this is perfectly valid
198219
// Content-Type: text/plain; charset = "iso-8859-1"
199220
//
221+
bool isQuoted = false;
200222
for (; valuePos < value.GetLength(); valuePos++)
201223
{
202224
char c = value[valuePos];
203225

204-
if (c == ' ' || c == '"')
226+
if (c == ' ')
205227
continue;
206-
else
207-
break;
228+
else if (c == '"') { isQuoted = true; valuePos++; break; }
229+
else break;
208230
}
209231

210-
// Locate the end of the value. The value may contain
211-
// pretty much any character, including space.
232+
// Locate the end of the value.
233+
// For quoted values scan to the closing '"'; for unquoted values scan
234+
// to the next ';' or '"'. This ensures semicolons inside quoted values
235+
// (e.g. filename="semi;colon.dll") are included in the result.
212236
int valueEndPos = valuePos;
213237
for (; valueEndPos < value.GetLength(); valueEndPos++)
214238
{
215239
char c = value[valueEndPos];
216240

217-
if (c == ';' || c == '"')
218-
break;
241+
if (isQuoted)
242+
{
243+
if (c == '"') break;
244+
}
219245
else
220-
continue;
246+
{
247+
if (c == ';' || c == '"') break;
248+
}
221249
}
222250

223251
int valueLength = valueEndPos - valuePos;
224252

225253
value = value.Mid(valuePos, valueLength);
226254

227255
// If the value is
228-
// Content-Type: text/plain; charset = "iso-8859-1"
256+
// Content-Type: text/plain; charset = "iso-8859-1"
229257
// it needs to be trimmed.
230258
value.TrimRight();
231259

@@ -262,6 +290,34 @@ namespace HM
262290
return true;
263291
}
264292

293+
// Remove all parameters whose base name matches pszAttr, including RFC 2231
294+
// continuation parameters (filename*0, filename*1, ...) and encoded variants (filename*).
295+
void MimeField::RemoveParameter(const char* pszAttr)
296+
{
297+
bool encodedParameter;
298+
int nPos, nSize;
299+
bool changed = false;
300+
301+
while (FindParameter(pszAttr, nPos, nSize, encodedParameter))
302+
{
303+
// nPos points to the value start (right after '=').
304+
// Walk backwards to find the ';' that begins this parameter segment.
305+
// rfind is safe here: any ';' inside a preceding quoted value is at a position
306+
// less than that value's opening '"', which is itself less than nPos.
307+
string::size_type segStart = value_.rfind(';', nPos);
308+
if (segStart == string::npos)
309+
break;
310+
311+
size_t segEnd = nPos + nSize;
312+
313+
value_.erase(segStart, segEnd - segStart);
314+
changed = true;
315+
}
316+
317+
if (changed)
318+
modified_ = true;
319+
}
320+
265321
int MimeField::GetLength() const
266322
{
267323
int nLength = (int) name_.size() + 4;
@@ -425,12 +481,13 @@ namespace HM
425481
const char* pszParmEnd = NULL;
426482
if (*pszParms == '"') // quoted string
427483
pszParmEnd = ::strchr(pszParms+1, '"');
428-
if (!pszParmEnd) // non quoted string
484+
if (!pszParmEnd) // non quoted string (includes RFC 2231 values like UTF-8''name)
429485
{
430486
pszParmEnd = pszParms;
431487

432-
// Locate end of parameter value.
433-
while (CMimeChar::IsToken(*pszParmEnd) || (*pszParmEnd == '.'))
488+
// Scan to the next ';' or end. Using IsToken here is insufficient because
489+
// RFC 2231 unquoted values contain non-token characters such as apostrophes.
490+
while (*pszParmEnd && *pszParmEnd != ';')
434491
pszParmEnd++;
435492
}
436493
else pszParmEnd++; // pszParmEnd -> end of parameter value
@@ -519,12 +576,26 @@ namespace HM
519576
{
520577
AnsiString encoded_filename = MIMEUnicodeEncoder::EncodeValue("utf-8", file_name);
521578

522-
AnsiString sRawValue = GetParameter(CMimeConst::ContentDisposition(), CMimeConst::Filename());
523-
if (!sRawValue.IsEmpty())
524-
SetParameter(CMimeConst::ContentDisposition(), CMimeConst::Filename(), encoded_filename);
525-
else
526-
SetParameter(CMimeConst::ContentType(), CMimeConst::Name(), encoded_filename);
527-
579+
MimeField* pfd = GetField(CMimeConst::ContentDisposition());
580+
if (pfd != nullptr)
581+
{
582+
AnsiString existingValue;
583+
if (pfd->GetParameter(CMimeConst::Filename(), existingValue))
584+
{
585+
// Remove all existing filename parameters, including RFC 2231 continuations
586+
// (filename*0, filename*1, filename*), before setting the new single value.
587+
pfd->RemoveParameter(CMimeConst::Filename());
588+
pfd->SetParameter(CMimeConst::Filename(), encoded_filename);
589+
return;
590+
}
591+
}
592+
593+
pfd = GetField(CMimeConst::ContentType());
594+
if (pfd != nullptr)
595+
{
596+
pfd->RemoveParameter(CMimeConst::Name());
597+
pfd->SetParameter(CMimeConst::Name(), encoded_filename);
598+
}
528599
}
529600

530601
String

‎hmailserver/source/Server/Common/Mime/Mime.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ namespace HM
9090

9191
void SetParameter(const char* pszAttr, const char* pszValue);
9292
bool GetParameter(const char* pszAttr, AnsiString& strValue) const;
93+
void RemoveParameter(const char* pszAttr);
9394
/*
9495
encodedParameter indicates whether the content is encoding according to
9596

‎hmailserver/source/Server/Common/Mime/MimeTester.cpp‎

Lines changed: 176 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -393,6 +393,157 @@ namespace HM
393393

394394
return true;
395395
}
396+
397+
// GetParameter must not split on ';' inside a quoted value.
398+
bool TestGetParameterWithQuotedSemicolon()
399+
{
400+
MimeField field;
401+
const char* line = "Content-Disposition: attachment; filename=\"semi;colon.dll\"\r\n";
402+
field.Load(line, strlen(line), false);
403+
404+
AnsiString value;
405+
if (!field.GetParameter("filename", value))
406+
return false;
407+
return value == "semi;colon.dll";
408+
}
409+
410+
// A quoted value containing ';' must not consume the parameter that follows it.
411+
bool TestGetParameterNeighboringParamUnaffectedByQuotedSemicolon()
412+
{
413+
MimeField field;
414+
const char* line = "Content-Disposition: attachment; filename=\"semi;colon.dll\"; size=42\r\n";
415+
field.Load(line, strlen(line), false);
416+
417+
AnsiString filename;
418+
if (!field.GetParameter("filename", filename))
419+
return false;
420+
if (filename != "semi;colon.dll")
421+
return false;
422+
423+
AnsiString size;
424+
if (!field.GetParameter("size", size))
425+
return false;
426+
return size == "42";
427+
}
428+
429+
// RemoveParameter must remove the named parameter while leaving others intact.
430+
bool TestRemoveParameterRemovesSimpleParam()
431+
{
432+
MimeField field;
433+
const char* line = "Content-Type: text/plain; charset=utf-8; name=\"test.txt\"\r\n";
434+
field.Load(line, strlen(line), false);
435+
436+
field.RemoveParameter("name");
437+
438+
AnsiString val;
439+
if (field.GetParameter("name", val))
440+
return false;
441+
442+
AnsiString charset;
443+
if (!field.GetParameter("charset", charset))
444+
return false;
445+
return charset == "utf-8";
446+
}
447+
448+
// RemoveParameter("filename") must remove RFC 2231 continuation parameters
449+
// (filename*0, filename*1) so that a subsequent SetParameter produces a clean
450+
// single value that GetParameter returns first.
451+
bool TestRemoveParameterRemovesRfc2231Continuations()
452+
{
453+
MimeField field;
454+
const char* line = "Content-Disposition: attachment; filename*0=\"long\"; filename*1=\"name.dll\"\r\n";
455+
field.Load(line, strlen(line), false);
456+
457+
field.RemoveParameter("filename");
458+
field.SetParameter("filename", "new.dll");
459+
460+
AnsiString val;
461+
if (!field.GetParameter("filename", val))
462+
return false;
463+
return val == "new.dll";
464+
}
465+
466+
// SetFileName must remove RFC 2231 continuation filename parameters before installing
467+
// the new single filename= value. With the old code the continuations were left
468+
// in place and shadowed the new value returned by GetParameter.
469+
bool TestSetFileNameReplacesRfc2231ContinuationFilename()
470+
{
471+
MimeHeader header;
472+
const char* headers =
473+
"Content-Type: application/octet-stream\r\n"
474+
"Content-Disposition: attachment; filename*0=\"oldpart0\"; filename*1=\"oldpart1\"\r\n"
475+
"\r\n";
476+
header.Load(headers, strlen(headers));
477+
478+
header.SetFileName(L"replacement.dll");
479+
480+
string result = header.GetParameter(CMimeConst::ContentDisposition(), CMimeConst::Filename());
481+
482+
if (result == "oldpart0" || result == "oldpart1")
483+
return false;
484+
485+
return result.find("replacement.dll") != string::npos;
486+
}
487+
488+
// SetFileName must also replace a single RFC 2231 encoded filename*= parameter
489+
// rather than appending a new filename= after it.
490+
bool TestSetFileNameReplacesEncodedFilenameVariant()
491+
{
492+
MimeHeader header;
493+
const char* headers =
494+
"Content-Type: application/octet-stream\r\n"
495+
"Content-Disposition: attachment; filename*=UTF-8''old%20name.dll\r\n"
496+
"\r\n";
497+
header.Load(headers, strlen(headers));
498+
499+
header.SetFileName(L"replacement.dll");
500+
501+
string result = header.GetParameter(CMimeConst::ContentDisposition(), CMimeConst::Filename());
502+
503+
if (result.find("old") != string::npos)
504+
return false;
505+
506+
return result.find("replacement.dll") != string::npos;
507+
}
508+
509+
// If there is no filename= on Content-Disposition, SetFileName falls back to
510+
// Content-Type name=. That path should also clear old RFC 2231-style name*
511+
// values before writing the new filename.
512+
bool TestSetFileNameReplacesContentTypeNameVariant()
513+
{
514+
MimeHeader header;
515+
const char* headers =
516+
"Content-Type: application/octet-stream; name*=UTF-8''old%20name.dll\r\n"
517+
"\r\n";
518+
header.Load(headers, strlen(headers));
519+
520+
header.SetFileName(L"replacement.dll");
521+
522+
string result = header.GetParameter(CMimeConst::ContentType(), CMimeConst::Name());
523+
524+
if (result.find("old") != string::npos)
525+
return false;
526+
527+
return result.find("replacement.dll") != string::npos;
528+
}
529+
530+
// An RFC 2231 unquoted value like filename*=UTF-8''hello.dll must not be truncated
531+
// at the apostrophes. The old code used IsToken which stops at non-token characters
532+
// such as apostrophe; the fix scans to ';' or end-of-string instead.
533+
bool TestRfc2231ApostropheInUnquotedValue()
534+
{
535+
MimeField field;
536+
const char* line = "Content-Disposition: attachment; filename*=UTF-8''hello.dll\r\n";
537+
field.Load(line, strlen(line), false);
538+
539+
AnsiString val;
540+
if (!field.GetParameter("filename", val))
541+
return false;
542+
// GetParameter decodes the RFC 2231 value; the decoded result must contain the
543+
// actual filename. With the old code the scan stopped at the first apostrophe
544+
// so only "UTF-8" was captured and "hello.dll" would be absent.
545+
return val.Find("hello.dll") >= 0;
546+
}
396547
}
397548

398549
MimeTester::MimeTester(void)
@@ -406,7 +557,7 @@ namespace HM
406557
void
407558
MimeTester::Test()
408559
{
409-
if (!TestFindStringEdgeCases())
560+
if (!TestFindStringEdgeCases())
410561
throw;
411562

412563
if (!TestMultipartWithoutFinalCrlf())
@@ -474,5 +625,29 @@ namespace HM
474625

475626
if (!TestQPEncodeNoTrailingWhitespaceBeforeSoftBreak())
476627
throw;
628+
if (!TestGetParameterWithQuotedSemicolon())
629+
throw;
630+
631+
if (!TestGetParameterNeighboringParamUnaffectedByQuotedSemicolon())
632+
throw;
633+
634+
if (!TestRemoveParameterRemovesSimpleParam())
635+
throw;
636+
637+
if (!TestRemoveParameterRemovesRfc2231Continuations())
638+
throw;
639+
640+
if (!TestSetFileNameReplacesRfc2231ContinuationFilename())
641+
throw;
642+
643+
if (!TestSetFileNameReplacesEncodedFilenameVariant())
644+
throw;
645+
646+
if (!TestSetFileNameReplacesContentTypeNameVariant())
647+
throw;
648+
649+
if (!TestRfc2231ApostropheInUnquotedValue())
650+
throw;
651+
477652
}
478653
}

0 commit comments

Comments
 (0)