Skip to content

Commit cbbe8d3

Browse files
rousskovsquid-anubis
authored andcommitted
Fix handling of truncated legacy errorpage %codes (#2411)
When build.input ends with a bare percent character, we must only copy/consume that character and increment build.input by 1, not 2. This overread bug existed since errorpage templates were introduced in 1997 commit 9b312a1. 2010 commit 4d16918 significantly broadened the kinds of use cases that can trigger this bug.
1 parent cb6ea84 commit cbbe8d3

1 file changed

Lines changed: 19 additions & 3 deletions

File tree

src/errorpage.cc

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -958,7 +958,8 @@ ErrorState::compileLegacyCode(Build &build)
958958

959959
const auto &building_deny_info_url = build.building_deny_info_url; // a change reduction hack
960960

961-
const auto letter = build.input[1];
961+
Assure(*build.input == '%');
962+
const auto letter = build.input[1]; // may be the terminating NUL
962963

963964
switch (letter) {
964965

@@ -1261,13 +1262,25 @@ ErrorState::compileLegacyCode(Build &build)
12611262
p = "%";
12621263
break;
12631264

1265+
case '\0':
1266+
// XXX: Partially duplicates error handling code of the `default:` case.
1267+
// TODO: Refactor bypassBuildErrorXXX() to accept `build` and determine the source of the error.
1268+
if (building_deny_info_url)
1269+
bypassBuildErrorXXX("Bare % at the end of deny_info", build.input);
1270+
else
1271+
bypassBuildErrorXXX("Bare % at the end of error page", build.input);
1272+
p = "%";
1273+
do_quote = 0;
1274+
break;
1275+
12641276
default:
12651277
if (building_deny_info_url)
12661278
bypassBuildErrorXXX("Unsupported deny_info %code", build.input);
12671279
else if (letter != ';')
12681280
bypassBuildErrorXXX("Unsupported error page %code", build.input);
12691281
// else too many "font-size: 100%;" template errors to report
12701282

1283+
Assure(build.input[1]);
12711284
mb.append(build.input, 2);
12721285
do_quote = 0;
12731286
break;
@@ -1278,7 +1291,8 @@ ErrorState::compileLegacyCode(Build &build)
12781291

12791292
assert(p);
12801293

1281-
debugs(4, 3, "%" << letter << " --> '" << p << "'" );
1294+
// TODO: Add an I/O manipulator to report non-printable chars better.
1295+
debugs(4, 3, "%" << (letter ? letter : '?') << " --> '" << p << "'" );
12821296

12831297
if (do_quote)
12841298
p = html_quote(p);
@@ -1288,7 +1302,9 @@ ErrorState::compileLegacyCode(Build &build)
12881302

12891303
// TODO: Optimize by replacing mb with direct build.output usage.
12901304
build.output.append(p, strlen(p));
1291-
build.input += 2;
1305+
++build.input; // skip the parsed % character
1306+
if (letter)
1307+
++build.input; // when it was present, skip the parsed letter after %
12921308
}
12931309

12941310
void

0 commit comments

Comments
 (0)