Created attachment 25510 [details] patch When the `gp_validate_path_len` function validates a path, it distinguishes between absolute and relative paths. In the case of relative paths, it will check the path with and without the current-directory-prefix ("foo" and "./foo"). The problem is that it doesn't take into account paths with a parent-directory-prefix. So a path like "../../foo" is also tested as "./../../foo" and if the current directory "./" is in the permitted paths, it will pass the check and you can access arbitrary files. The fix is simple, I added a test for a path with the parent directory prefix.
Created attachment 25511 [details] testcase ``` gs -P curdirtravers.ps gs -I./ curdirtravers.ps gs --permit-file-XXX=./* curdirtravers.ps ```
It's been a busy few weeks, and it still hasn't settled down yet, so I'm commenting on all the open bug threads to reassure people that we are working on these. Due to slow responses on one thread it we are unfortunately not yet able to estimate a patch release date. Because we know that there is at least one person watching/mining our public repositories for security commits we are keeping these in a private repository until a patch release. I'm afraid that means I can't just point to a proposed commit in Gitweb. The patch for this issue is : diff --git a/base/gpmisc.c b/base/gpmisc.c index 59423584a..7d102d386 100644 --- a/base/gpmisc.c +++ b/base/gpmisc.c @@ -1042,7 +1042,7 @@ gp_validate_path_len(const gs_memory_t *mem, const uint len, const char *mode) { - char *buffer, *bufferfull; + char *buffer, *bufferfull = NULL; uint rlen; int code = 0; const char *cdirstr = gp_file_name_current(); @@ -1097,8 +1097,10 @@ gp_validate_path_len(const gs_memory_t *mem, return gs_error_VMerror; buffer = bufferfull + prefix_len; - if (gp_file_name_reduce(path, (uint)len, buffer, &rlen) != gp_combine_success) - return gs_error_invalidfileaccess; + if (gp_file_name_reduce(path, (uint)len, buffer, &rlen) != gp_combine_success) { + code = gs_note_error(gs_error_invalidfileaccess); + goto exit; + } buffer[rlen] = 0; } while (1) { @@ -1133,9 +1135,34 @@ gp_validate_path_len(const gs_memory_t *mem, code = gs_note_error(gs_error_invalidfileaccess); } if (code < 0 && prefix_len > 0 && buffer > bufferfull) { + uint newlen = rlen + cdirstrl + dirsepstrl; + char *newbuffer; + int code; + buffer = bufferfull; memcpy(buffer, cdirstr, cdirstrl); memcpy(buffer + cdirstrl, dirsepstr, dirsepstrl); + + /* We've prepended a './' or similar for the current working directory. We need + * to execute file_name_reduce on that, to eliminate any '../' or similar from + * the (new) full path. + */ + newbuffer = (char *)gs_alloc_bytes(mem->thread_safe_memory, newlen + 1, "gp_validate_path"); + if (newbuffer == NULL) { + code = gs_note_error(gs_error_VMerror); + goto exit; + } + + memcpy(newbuffer, buffer, rlen + cdirstrl + dirsepstrl); + newbuffer[newlen] = 0x00; + + code = gp_file_name_reduce(newbuffer, (uint)newlen, buffer, &newlen); + gs_free_object(mem->thread_safe_memory, newbuffer, "gp_validate_path"); + if (code != gp_combine_success) { + code = gs_note_error(gs_error_invalidfileaccess); + goto exit; + } + continue; } else if (code < 0 && cdirstrl > 0 && prefix_len == 0 && buffer == bufferfull) { @@ -1154,6 +1181,7 @@ gp_validate_path_len(const gs_memory_t *mem, gs_path_control_flag_is_scratch_file); } +exit: gs_free_object(mem->thread_safe_memory, bufferfull, "gp_validate_path"); #ifdef EACCES if (code == gs_error_invalidfileaccess)
Right now I'm not on a computer where I can compile and test it, but I think it fixes the vulnerability. But you don't need to reduce the path twice and I don't like that the code is sort of "algorithmically" still wrong. * It looks at the cwd prefix before the path is reduced. After the reduction, the path is changed so the previous look at the prefix is irrelevant. * The path variants with and without cwd prefix is still tried on all non-absolute paths. I would like it more like this ``` if it is a pipe: leave the path as is elif it is a weird pipe-like path: reject it (done in the other patch) else: first of all reduce the path if and only iff it is a cwd path: fiddle with the cwd prefix ```
Use CVE-2024-33870