From 4fe75a3a0f41ddc528b09dd9079d75a322781a07 Mon Sep 17 00:00:00 2001 From: Kevin Day Date: Sun, 2 Jan 2022 12:43:36 -0600 Subject: [PATCH] Bugfix: Incorrect mode is being set. The world/other character is being mixed with the user. Swap the "on" value for world/other and user. Always reset the "what" value before starting the condition loop. Turns out that the previous code does not support the form "u+rw-x". The following is an alternative way to write this that previously worked "u+rw,u-x". Make sure both methods are supported by adding an additional check for the "+", "-", and "=". When adding and removing, the previous opposing bits needs to be reset. For example "u+rwx-r" should set the "add read" bit and then remove the "add read" bit while setting the "subtract read" bit. Also absurd forms like the following need to work: "u+rrrrrwx+rwwwxwrwrwrwww" or "u+rwx,g-rwx+wrrr,o+rwx-wwr". A copy and paste mistake in f_file_mode_to_mode() results in the wrong bits being set for the world bits. Don't operate on the parameters directly. Update them only on success. This ensures a safer design at a cost of slightly more resources being used. Swap the world and the owner bits for replace to make it more logically consistent in the order of the bits. Hopefully this will make things slightly less confusing. --- level_0/f_file/c/file-common.h | 4 +- level_0/f_file/c/file.c | 120 +++++++++++++++++++++++++++-------------- 2 files changed, 82 insertions(+), 42 deletions(-) diff --git a/level_0/f_file/c/file-common.h b/level_0/f_file/c/file-common.h index 0cc7ef4..e985584 100644 --- a/level_0/f_file/c/file-common.h +++ b/level_0/f_file/c/file-common.h @@ -321,9 +321,9 @@ extern "C" { #define F_file_mode_t_mask_bit_sticky_d 0x11000000 // 0001 0001 0000 0000 0000 0000 0000 0000 #define F_file_mode_t_mask_bit_write_d 0x00222222 // 0000 0000 0010 0010 0010 0010 0010 0010 - #define F_file_mode_t_replace_owner_d 0x1 // 0000 0001 + #define F_file_mode_t_replace_world_d 0x1 // 0000 0001 #define F_file_mode_t_replace_group_d 0x2 // 0000 0010 - #define F_file_mode_t_replace_world_d 0x4 // 0000 0100 + #define F_file_mode_t_replace_owner_d 0x4 // 0000 0100 #define F_file_mode_t_replace_special_d 0x8 // 0000 1000 #define F_file_mode_t_replace_directory_d 0x10 // 0001 0000 diff --git a/level_0/f_file/c/file.c b/level_0/f_file/c/file.c index eec25d1..534513c 100644 --- a/level_0/f_file/c/file.c +++ b/level_0/f_file/c/file.c @@ -734,7 +734,6 @@ extern "C" { *mode |= mode_file & F_file_mode_group_rwx_d; if (mode_change & F_file_mode_t_block_group_d) { - if (change & F_file_mode_t_mask_bit_read_d & F_file_mode_t_mask_how_subtract_d) { if (*mode & F_file_mode_group_r_d) { *mode -= F_file_mode_group_r_d; @@ -793,7 +792,6 @@ extern "C" { change = mode_change & F_file_mode_t_block_world_d; if (mode_replace & F_file_mode_t_replace_world_d) { - if (change & F_file_mode_t_mask_bit_read_d & F_file_mode_t_mask_how_add_d) { *mode |= F_file_mode_world_r_d; } @@ -815,7 +813,6 @@ extern "C" { *mode |= mode_file & F_file_mode_world_rwx_d; if (mode_change & F_file_mode_t_block_world_d) { - if (change & F_file_mode_t_mask_bit_read_d & F_file_mode_t_mask_how_subtract_d) { if (*mode & F_file_mode_world_r_d) { *mode -= F_file_mode_world_r_d; @@ -885,12 +882,10 @@ extern "C" { #endif // _di_level_0_parameter_checking_ uint8_t syntax = 0; - - *mode = 0; - *replace = 0; + uint8_t replace_result = 0; + f_file_mode_t mode_result = 0; if (string[0] == f_string_ascii_plus_s[0] || string[0] == f_string_ascii_minus_s[0] || string[0] == f_string_ascii_equal_s[0]) { - if (string[1] == f_string_ascii_r_s[0] || f_string_ascii_w_s[0] || f_string_ascii_x_s[0] || f_string_ascii_X_s[0] || f_string_ascii_s_s[0] ||f_string_ascii_t_s[0]) { syntax = 1; } @@ -913,7 +908,7 @@ extern "C" { if (syntax == 1) { uint8_t on = 0; // 1 = user, 2 = group, 4 = world/sticky, 7 = all. - uint8_t how = 0; // 1 = add, 2 = replace, 3 = subtract. + uint8_t how = 0; // 1 = add, 2 = replace, 3 = subtract, 4 = add when no "on", 5 = replace when no "on", and 6 = subtract when no "on". f_file_mode_t mode_mask = 0; f_file_mode_t mode_umask = 0; @@ -970,17 +965,17 @@ extern "C" { for (f_array_length_t i = 0; syntax && string[i]; ++i) { - if (string[i] == f_string_ascii_o_s[0]) { + if (string[i] == f_string_ascii_u_s[0]) { on |= 1; - mode_mask |= F_file_mode_t_block_world_d; + mode_mask |= F_file_mode_t_block_owner_d; } else if (string[i] == f_string_ascii_g_s[0]) { on |= 2; mode_mask |= F_file_mode_t_block_group_d; } - else if (string[i] == f_string_ascii_u_s[0]) { + else if (string[i] == f_string_ascii_o_s[0]) { on |= 4; - mode_mask |= F_file_mode_t_block_owner_d; + mode_mask |= F_file_mode_t_block_world_d; } else if (string[i] == f_string_ascii_a_s[0]) { on = 7; @@ -996,22 +991,22 @@ extern "C" { else { how = on ? 2 : 5; - // clear by mask to prepare for replacement, which includes clearing the special block. + // Clear by mask to prepare for replacement, which includes clearing the special block. mode_mask |= F_file_mode_t_block_special_d; - *mode -= (*mode) & mode_mask; + mode_result -= mode_result & mode_mask; - *replace |= F_file_mode_t_replace_special_d; + replace_result |= F_file_mode_t_replace_special_d; if (mode_mask & F_file_mode_t_block_owner_d) { - *replace |= F_file_mode_t_replace_owner_d; + replace_result |= F_file_mode_t_replace_owner_d; } if (mode_mask & F_file_mode_t_block_group_d) { - *replace |= F_file_mode_t_replace_group_d; + replace_result |= F_file_mode_t_replace_group_d; } if (mode_mask & F_file_mode_t_block_world_d) { - *replace |= F_file_mode_t_replace_world_d; + replace_result |= F_file_mode_t_replace_world_d; } } @@ -1020,6 +1015,8 @@ extern "C" { mode_mask = F_file_mode_t_block_all_d; } + what = 0; + for (++i; string[i]; ++i) { if (string[i] == f_string_ascii_r_s[0]) { @@ -1037,7 +1034,7 @@ extern "C" { else if (string[i] == f_string_ascii_s_s[0]) { mode_mask |= F_file_mode_t_block_special_d; - if (on & 4) { + if (on & 1) { what = F_file_mode_t_mask_bit_set_owner_d; } else if (on & 2) { @@ -1050,7 +1047,7 @@ extern "C" { else if (string[i] == f_string_ascii_t_s[0]) { mode_mask |= F_file_mode_t_block_special_d; - if (on & 1) { + if (on & 4) { what = F_file_mode_t_mask_bit_sticky_d; } else { @@ -1059,7 +1056,7 @@ extern "C" { } else if (string[i] == f_string_ascii_comma_s[0]) { if (how > 3) { - *mode -= *mode & mode_umask; + mode_result -= mode_result & mode_umask; } on = 0; @@ -1068,22 +1065,63 @@ extern "C" { break; } + else if (string[i] == f_string_ascii_plus_s[0] || string[i] == f_string_ascii_minus_s[0] || string[i] == f_string_ascii_equal_s[0]) { + what = 0; + + if (string[i] == f_string_ascii_plus_s[0]) { + how = on ? 1 : 4; + } + else if (string[i] == f_string_ascii_minus_s[0]) { + how = on ? 3 : 6; + } + else { + how = on ? 2 : 5; + + // Clear by mask to prepare for replacement, which includes clearing the special block. + mode_mask |= F_file_mode_t_block_special_d; + mode_result -= mode_result & mode_mask; + + replace_result |= F_file_mode_t_replace_special_d; + + if (mode_mask & F_file_mode_t_block_owner_d) { + replace_result |= F_file_mode_t_replace_owner_d; + } + + if (mode_mask & F_file_mode_t_block_group_d) { + replace_result |= F_file_mode_t_replace_group_d; + } + + if (mode_mask & F_file_mode_t_block_world_d) { + replace_result |= F_file_mode_t_replace_world_d; + } + } + } else { syntax = 0; break; } - if (how == 1 || how == 2 || how == 4 || how == 5) { - *mode |= what & mode_mask & F_file_mode_t_mask_how_add_d; - } - else if (how == 3 || how == 6) { - *mode |= what & mode_mask & F_file_mode_t_mask_how_subtract_d; + if (what) { + if (how == 1 || how == 2 || how == 4 || how == 5) { + mode_result |= what & mode_mask & F_file_mode_t_mask_how_add_d; + + if (mode_result & what & mode_mask & F_file_mode_t_mask_how_subtract_d) { + mode_result -= mode_result & what & mode_mask & F_file_mode_t_mask_how_subtract_d; + } + } + else if (how == 3 || how == 6) { + mode_result |= what & mode_mask & F_file_mode_t_mask_how_subtract_d; + + if (mode_result & what & mode_mask & F_file_mode_t_mask_how_add_d) { + mode_result -= mode_result & what & mode_mask & F_file_mode_t_mask_how_add_d; + } + } } } // for if (how > 3) { - *mode -= *mode & mode_umask; + mode_result -= mode_result & mode_umask; } if (!string[i]) break; @@ -1098,9 +1136,11 @@ extern "C" { // 1 = add, 2 = replace, 3 = subtract. uint8_t how = 0; - f_array_length_t i = 0; + // 0 = none, 0x1 = leading zero. + uint8_t option = 0; - *replace = 0; + f_array_length_t i = 0; + f_array_length_t j = 0; if (string[0] == f_string_ascii_plus_s[0]) { how = 1; @@ -1114,12 +1154,12 @@ extern "C" { how = 2; i = 1; - *replace = F_file_mode_t_replace_standard_d; + replace_result = F_file_mode_t_replace_standard_d; } else { how = 2; - *replace = F_file_mode_t_replace_standard_d | F_file_mode_t_replace_directory_d; + replace_result = F_file_mode_t_replace_standard_d | F_file_mode_t_replace_directory_d; } // Seek past leading '0's. @@ -1133,7 +1173,7 @@ extern "C" { for (; string[i + j] && j < 4; ++j) { if (j) { - *mode <<= 8; + mode_result <<= 8; } if (string[i] == f_string_ascii_0_s[0]) { @@ -1143,10 +1183,10 @@ extern "C" { // This assumes ASCII/UTF-8. if (how == 3) { - *mode |= (string[i + j] - 0x30) << 4; + mode_result |= (string[i + j] - 0x30) << 4; } else { - *mode |= string[i + j] - 0x30; + mode_result |= string[i + j] - 0x30; } } else { @@ -1163,20 +1203,20 @@ extern "C" { else if (how == 2) { // If there are only '0's then the standard and setuid/setgid/sticky bits are to be replaced. - if (!*mode) { - *replace = F_file_mode_t_replace_standard_d | F_file_mode_t_replace_special_d; + if (!mode_result) { + replace_result = F_file_mode_t_replace_standard_d | F_file_mode_t_replace_special_d; } } } } if (syntax) { + *mode = mode_result; + *replace = replace_result; + return F_none; } - *mode = 0; - *replace = 0; - return F_status_set_error(F_syntax); } #endif // _di_f_file_mode_from_string_ @@ -1275,7 +1315,7 @@ extern "C" { *to |= F_file_mode_group_r_d; } - if (add & F_file_mode_t_block_world_d & F_file_mode_t_mask_bit_write_d) { + if (add & F_file_mode_t_block_world_d & F_file_mode_t_mask_bit_read_d) { *to |= F_file_mode_world_r_d; } -- 1.8.3.1