Skip to content

Commit 5f56c6f

Browse files
committed
Bug 706761: Don't "reduce" %pipe% file names for permission validation
For regular file names, we try to simplfy relative paths before we use them. Because the %pipe% device can, effectively, accept command line calls, we shouldn't be simplifying that string, because the command line syntax can end up confusing the path simplifying code. That can result in permitting a pipe command which does not match what was originally permitted. Special case "%pipe" in the validation code so we always deal with the entire string. Bug 706778: 706761 revisit Two problems with the original commit. The first a silly typo inverting the logic of a test. The second was forgetting that we actually actually validate two candidate strings for pipe devices. One with the expected "%pipe%" prefix, the other using the pipe character prefix: "|". This addresses both those.
1 parent 8d53dad commit 5f56c6f

File tree

2 files changed

+64
-23
lines changed

2 files changed

+64
-23
lines changed

base/gpmisc.c

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1076,16 +1076,29 @@ gp_validate_path_len(const gs_memory_t *mem,
10761076
&& !memcmp(path + cdirstrl, dirsepstr, dirsepstrl)) {
10771077
prefix_len = 0;
10781078
}
1079-
rlen = len+1;
1080-
bufferfull = (char *)gs_alloc_bytes(mem->thread_safe_memory, rlen + prefix_len, "gp_validate_path");
1081-
if (bufferfull == NULL)
1082-
return gs_error_VMerror;
1083-
1084-
buffer = bufferfull + prefix_len;
1085-
if (gp_file_name_reduce(path, (uint)len, buffer, &rlen) != gp_combine_success)
1086-
return gs_error_invalidfileaccess;
1087-
buffer[rlen] = 0;
10881079

1080+
/* "%pipe%" do not follow the normal rules for path definitions, so we
1081+
don't "reduce" them to avoid unexpected results
1082+
*/
1083+
if (path[0] == '|' || (len > 5 && memcmp(path, "%pipe", 5) == 0)) {
1084+
bufferfull = buffer = (char *)gs_alloc_bytes(mem->thread_safe_memory, len + 1, "gp_validate_path");
1085+
if (buffer == NULL)
1086+
return gs_error_VMerror;
1087+
memcpy(buffer, path, len);
1088+
buffer[len] = 0;
1089+
rlen = len;
1090+
}
1091+
else {
1092+
rlen = len+1;
1093+
bufferfull = (char *)gs_alloc_bytes(mem->thread_safe_memory, rlen + prefix_len, "gp_validate_path");
1094+
if (bufferfull == NULL)
1095+
return gs_error_VMerror;
1096+
1097+
buffer = bufferfull + prefix_len;
1098+
if (gp_file_name_reduce(path, (uint)len, buffer, &rlen) != gp_combine_success)
1099+
return gs_error_invalidfileaccess;
1100+
buffer[rlen] = 0;
1101+
}
10891102
while (1) {
10901103
switch (mode[0])
10911104
{

base/gslibctx.c

Lines changed: 42 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -740,14 +740,28 @@ gs_add_control_path_len_flags(const gs_memory_t *mem, gs_path_control_t type, co
740740
return gs_error_rangecheck;
741741
}
742742

743-
rlen = len+1;
744-
buffer = (char *)gs_alloc_bytes(core->memory, rlen, "gp_validate_path");
745-
if (buffer == NULL)
746-
return gs_error_VMerror;
743+
/* "%pipe%" do not follow the normal rules for path definitions, so we
744+
don't "reduce" them to avoid unexpected results
745+
*/
746+
if (path[0] == '|' || (len > 5 && memcmp(path, "%pipe", 5) == 0)) {
747+
buffer = (char *)gs_alloc_bytes(core->memory, len + 1, "gs_add_control_path_len");
748+
if (buffer == NULL)
749+
return gs_error_VMerror;
750+
memcpy(buffer, path, len);
751+
buffer[len] = 0;
752+
rlen = len;
753+
}
754+
else {
755+
rlen = len + 1;
747756

748-
if (gp_file_name_reduce(path, (uint)len, buffer, &rlen) != gp_combine_success)
749-
return gs_error_invalidfileaccess;
750-
buffer[rlen] = 0;
757+
buffer = (char *)gs_alloc_bytes(core->memory, rlen, "gs_add_control_path_len");
758+
if (buffer == NULL)
759+
return gs_error_VMerror;
760+
761+
if (gp_file_name_reduce(path, (uint)len, buffer, &rlen) != gp_combine_success)
762+
return gs_error_invalidfileaccess;
763+
buffer[rlen] = 0;
764+
}
751765

752766
n = control->num;
753767
for (i = 0; i < n; i++)
@@ -833,14 +847,28 @@ gs_remove_control_path_len_flags(const gs_memory_t *mem, gs_path_control_t type,
833847
return gs_error_rangecheck;
834848
}
835849

836-
rlen = len+1;
837-
buffer = (char *)gs_alloc_bytes(core->memory, rlen, "gp_validate_path");
838-
if (buffer == NULL)
839-
return gs_error_VMerror;
850+
/* "%pipe%" do not follow the normal rules for path definitions, so we
851+
don't "reduce" them to avoid unexpected results
852+
*/
853+
if (path[0] == '|' || (len > 5 && memcmp(path, "%pipe", 5) == 0)) {
854+
buffer = (char *)gs_alloc_bytes(core->memory, len + 1, "gs_remove_control_path_len");
855+
if (buffer == NULL)
856+
return gs_error_VMerror;
857+
memcpy(buffer, path, len);
858+
buffer[len] = 0;
859+
rlen = len;
860+
}
861+
else {
862+
rlen = len+1;
840863

841-
if (gp_file_name_reduce(path, (uint)len, buffer, &rlen) != gp_combine_success)
842-
return gs_error_invalidfileaccess;
843-
buffer[rlen] = 0;
864+
buffer = (char *)gs_alloc_bytes(core->memory, rlen, "gs_remove_control_path_len");
865+
if (buffer == NULL)
866+
return gs_error_VMerror;
867+
868+
if (gp_file_name_reduce(path, (uint)len, buffer, &rlen) != gp_combine_success)
869+
return gs_error_invalidfileaccess;
870+
buffer[rlen] = 0;
871+
}
844872

845873
n = control->num;
846874
for (i = 0; i < n; i++) {

0 commit comments

Comments
 (0)