From 1d949c82ca464b090b8afb0f088efc5fd97c97ef Mon Sep 17 00:00:00 2001 From: Jordan Bracco Date: Sun, 14 Jun 2020 18:21:17 +0200 Subject: [PATCH] Safeguard against ei_* failures --- lib/gen_magic/server.ex | 2 + src/apprentice.c | 78 ++++++++++++++++-------------- test/gen_magic/apprentice_test.exs | 68 ++++++++++++++------------ 3 files changed, 80 insertions(+), 68 deletions(-) diff --git a/lib/gen_magic/server.ex b/lib/gen_magic/server.ex index f057dda..b9a3d22 100644 --- a/lib/gen_magic/server.ex +++ b/lib/gen_magic/server.ex @@ -198,6 +198,8 @@ defmodule GenMagic.Server do 1 -> :no_database 2 -> :no_argument 3 -> :missing_database + 4 -> :ei_alloc_failed + 5 -> :ei_bad_term code -> {:unexpected_error, code} end diff --git a/src/apprentice.c b/src/apprentice.c index 9fb76be..806a288 100644 --- a/src/apprentice.c +++ b/src/apprentice.c @@ -58,8 +58,8 @@ #define ERROR_NO_DATABASE 1 #define ERROR_NO_ARGUMENT 2 #define ERROR_MISSING_DATABASE 3 -#define ERROR_BAD_TERM 4 -#define ERROR_EI 5 +#define ERROR_EI 4 +#define ERROR_BAD_TERM 5 // We use a bigger than possible valid command length (around 4111 bytes) to // allow more precise errors when using too long paths. @@ -69,6 +69,13 @@ #define MAGIC_FLAGS_COMMON (MAGIC_CHECK | MAGIC_ERROR) magic_t magic_setup(int flags); +#define EI_ENSURE(result) \ + do { \ + if (result != 0) { \ + exit(ERROR_EI); \ + } \ + } while (0); + typedef char byte; void setup_environment(); @@ -103,11 +110,10 @@ int main(int argc, char **argv) { setup_system(); ei_x_buff ok_buf; - if (ei_x_new_with_version(&ok_buf) || ei_x_encode_atom(&ok_buf, "ready")) - exit(ERROR_EI); + EI_ENSURE(ei_x_new_with_version(&ok_buf)); + EI_ENSURE(ei_x_encode_atom(&ok_buf, "ready")); write_cmd(ok_buf.buff, ok_buf.index); - if (ei_x_free(&ok_buf) != 0) - exit(ERROR_EI); + EI_ENSURE(ei_x_free(&ok_buf)); byte buf[COMMAND_BUFFER_SIZE]; uint16_t len; @@ -125,9 +131,8 @@ int process_command(uint16_t len, byte *buf) { index = 0; // Initialize result - if (ei_x_new_with_version(&result) || ei_x_encode_tuple_header(&result, 2)) { - exit(ERROR_EI); - } + EI_ENSURE(ei_x_new_with_version(&result)); + EI_ENSURE(ei_x_encode_tuple_header(&result, 2)); if (len >= COMMAND_LEN) { error(&result, "badarg"); @@ -160,7 +165,7 @@ int process_command(uint16_t len, byte *buf) { if (termtype == ERL_BINARY_EXT) { if (termsize < 4096) { long bin_length; - ei_decode_binary(buf, &index, path, &bin_length); + EI_ENSURE(ei_decode_binary(buf, &index, path, &bin_length)); path[termsize] = '\0'; process_file(path, &result); } else { @@ -175,11 +180,11 @@ int process_command(uint16_t len, byte *buf) { int termtype; int termsize; char bytes[51]; - ei_get_type(buf, &index, &termtype, &termsize); + EI_ENSURE(ei_get_type(buf, &index, &termtype, &termsize)); if (termtype == ERL_BINARY_EXT && termsize < 50) { long bin_length; - ei_decode_binary(buf, &index, bytes, &bin_length); + EI_ENSURE(ei_decode_binary(buf, &index, bytes, &bin_length)); bytes[termsize] = '\0'; process_bytes(bytes, termsize, &result); } else { @@ -195,9 +200,7 @@ int process_command(uint16_t len, byte *buf) { write_cmd(result.buff, result.index); - if (ei_x_free(&result) != 0) { - exit(ERROR_EI); - } + EI_ENSURE(ei_x_free(&result)); return 0; } @@ -254,7 +257,6 @@ void setup_options_file(char *optarg) { } void setup_options_default() { - struct magic_file *next = malloc(sizeof(struct magic_file)); next->path = NULL; next->prev = magic_database; @@ -314,22 +316,24 @@ void process_bytes(char *path, int size, ei_x_buff *result) { return; } - ei_x_encode_atom(result, "ok"); - ei_x_encode_tuple_header(result, 3); - ei_x_encode_binary(result, mime_type_result, strlen(mime_type_result)); - ei_x_encode_binary(result, mime_encoding_result, - strlen(mime_encoding_result)); - ei_x_encode_binary(result, type_name_result, strlen(type_name_result)); + EI_ENSURE(ei_x_encode_atom(result, "ok")); + EI_ENSURE(ei_x_encode_tuple_header(result, 3)); + EI_ENSURE( + ei_x_encode_binary(result, mime_type_result, strlen(mime_type_result))); + EI_ENSURE(ei_x_encode_binary(result, mime_encoding_result, + strlen(mime_encoding_result))); + EI_ENSURE( + ei_x_encode_binary(result, type_name_result, strlen(type_name_result))); return; } void handle_magic_error(magic_t handle, int errn, ei_x_buff *result) { const char *error = magic_error(handle); - ei_x_encode_atom(result, "error"); - ei_x_encode_tuple_header(result, 2); + EI_ENSURE(ei_x_encode_atom(result, "error")); + EI_ENSURE(ei_x_encode_tuple_header(result, 2)); long errlon = (long)errn; - ei_x_encode_long(result, errlon); - ei_x_encode_binary(result, error, strlen(error)); + EI_ENSURE(ei_x_encode_long(result, errlon)); + EI_ENSURE(ei_x_encode_binary(result, error, strlen(error))); return; } @@ -358,12 +362,14 @@ void process_file(char *path, ei_x_buff *result) { return; } - ei_x_encode_atom(result, "ok"); - ei_x_encode_tuple_header(result, 3); - ei_x_encode_binary(result, mime_type_result, strlen(mime_type_result)); - ei_x_encode_binary(result, mime_encoding_result, - strlen(mime_encoding_result)); - ei_x_encode_binary(result, type_name_result, strlen(type_name_result)); + EI_ENSURE(ei_x_encode_atom(result, "ok")); + EI_ENSURE(ei_x_encode_tuple_header(result, 3)); + EI_ENSURE( + ei_x_encode_binary(result, mime_type_result, strlen(mime_type_result))); + EI_ENSURE(ei_x_encode_binary(result, mime_encoding_result, + strlen(mime_encoding_result))); + EI_ENSURE( + ei_x_encode_binary(result, type_name_result, strlen(type_name_result))); return; } @@ -428,12 +434,10 @@ size_t write_cmd(byte *buf, size_t len) { } void error(ei_x_buff *result, const char *error) { - ei_x_encode_atom(result, "error"); - ei_x_encode_atom(result, error); + EI_ENSURE(ei_x_encode_atom(result, "error")); + EI_ENSURE(ei_x_encode_atom(result, error)); write_cmd(result->buff, result->index); - - if (ei_x_free(result) != 0) - exit(ERROR_EI); + EI_ENSURE(ei_x_free(result)); } void fdseek(uint16_t count) { diff --git a/test/gen_magic/apprentice_test.exs b/test/gen_magic/apprentice_test.exs index 7c6b4b5..8f50ea8 100644 --- a/test/gen_magic/apprentice_test.exs +++ b/test/gen_magic/apprentice_test.exs @@ -2,6 +2,7 @@ defmodule GenMagic.ApprenticeTest do use GenMagic.MagicCase @tmp_path "/tmp/testgenmagicx" + require Logger test "sends ready" do port = Port.open(GenMagic.Config.get_port_name(), GenMagic.Config.get_port_options([])) @@ -43,7 +44,7 @@ defmodule GenMagic.ApprenticeTest do test "exits with badly formatted erlang terms", %{port: port} do send(port, {self(), {:command, "i forgot to term_to_binary!!"}}) - assert_receive {^port, {:exit_status, 4}} + assert_receive {^port, {:exit_status, 5}} end test "errors with wrong command", %{port: port} do @@ -89,37 +90,42 @@ defmodule GenMagic.ApprenticeTest do test "works with big file path", %{port: port} do # Test with longest valid path. {dir, bigfile} = too_big(@tmp_path, "/a") - File.mkdir_p!(dir) - File.touch!(bigfile) - on_exit(fn -> File.rm_rf!(@tmp_path) end) - send(port, {self(), {:command, :erlang.term_to_binary({:file, bigfile})}}) - assert_receive {^port, {:data, data}} - assert {:ok, _} = :erlang.binary_to_term(data) - refute_receive _ + case File.mkdir_p(dir) do + :ok -> + File.touch!(bigfile) + on_exit(fn -> File.rm_rf!(@tmp_path) end) + send(port, {self(), {:command, :erlang.term_to_binary({:file, bigfile})}}) + assert_receive {^port, {:data, data}} + assert {:ok, _} = :erlang.binary_to_term(data) + refute_receive _ - # This path should be long enough for buffers, but larger than a valid path name. Magic will return an errno 36. - file = @tmp_path <> String.duplicate("a", 256) - send(port, {self(), {:command, :erlang.term_to_binary({:file, file})}}) - assert_receive {^port, {:data, data}} - assert {:error, {36, _}} = :erlang.binary_to_term(data) - refute_receive _ - # Theses filename should be too big for the path buffer. - file = bigfile <> "aaaaaaaaaa" - send(port, {self(), {:command, :erlang.term_to_binary({:file, file})}}) - assert_receive {^port, {:data, data}} - assert {:error, :enametoolong} = :erlang.binary_to_term(data) - refute_receive _ - # This call should be larger than the COMMAND_BUFFER_SIZE. Ensure nothing bad happens! - file = String.duplicate(bigfile, 4) - send(port, {self(), {:command, :erlang.term_to_binary({:file, file})}}) - assert_receive {^port, {:data, data}} - assert {:error, :badarg} = :erlang.binary_to_term(data) - refute_receive _ - # We re-run a valid call to ensure the buffer/... haven't been corrupted in port land. - send(port, {self(), {:command, :erlang.term_to_binary({:file, bigfile})}}) - assert_receive {^port, {:data, data}} - assert {:ok, _} = :erlang.binary_to_term(data) - refute_receive _ + # This path should be long enough for buffers, but larger than a valid path name. Magic will return an errno 36. + file = @tmp_path <> String.duplicate("a", 256) + send(port, {self(), {:command, :erlang.term_to_binary({:file, file})}}) + assert_receive {^port, {:data, data}} + assert {:error, {36, _}} = :erlang.binary_to_term(data) + refute_receive _ + # Theses filename should be too big for the path buffer. + file = bigfile <> "aaaaaaaaaa" + send(port, {self(), {:command, :erlang.term_to_binary({:file, file})}}) + assert_receive {^port, {:data, data}} + assert {:error, :enametoolong} = :erlang.binary_to_term(data) + refute_receive _ + # This call should be larger than the COMMAND_BUFFER_SIZE. Ensure nothing bad happens! + file = String.duplicate(bigfile, 4) + send(port, {self(), {:command, :erlang.term_to_binary({:file, file})}}) + assert_receive {^port, {:data, data}} + assert {:error, :badarg} = :erlang.binary_to_term(data) + refute_receive _ + # We re-run a valid call to ensure the buffer/... haven't been corrupted in port land. + send(port, {self(), {:command, :erlang.term_to_binary({:file, bigfile})}}) + assert_receive {^port, {:data, data}} + assert {:ok, _} = :erlang.binary_to_term(data) + refute_receive _ + {:error, :enametoolong} -> + Logger.info("Skipping test, operating system does not support max POSIX length for directories") + :ignore + end end end