From 095d68fad08c223bc924e63ce9ac1021bcbc2feb Mon Sep 17 00:00:00 2001 From: patrick96 Date: Mon, 30 Apr 2018 18:45:01 +0200 Subject: [PATCH] fix: Stop using ato* for string to num conversion atoi, atof and so on have undefined behavior if anything goes wrong. We now use strto*, but without error checking. In most places overflows and the like *should* not happen. String to number conversions are only used when reading data from other applications or from the config, if another application gives unparsable strings or too large numbers, then most likely there is something wrong with that application. If the error comes from the user config, then the user has to live with values provided by strto* on error (which are very reasonable) Fixes #1201 --- src/components/bar.cpp | 16 ++++++++-------- src/components/builder.cpp | 2 +- src/components/config.cpp | 4 ++-- src/components/parser.cpp | 2 +- src/components/renderer.cpp | 2 +- src/ipc.cpp | 2 +- src/modules/menu.cpp | 4 ++-- src/modules/mpd.cpp | 6 +++--- src/modules/temperature.cpp | 2 +- src/x11/tray_manager.cpp | 4 ++-- 10 files changed, 22 insertions(+), 22 deletions(-) diff --git a/src/components/bar.cpp b/src/components/bar.cpp index e874fad5..1abea440 100644 --- a/src/components/bar.cpp +++ b/src/components/bar.cpp @@ -248,23 +248,23 @@ bar::bar(connection& conn, signal_emitter& emitter, const config& config, const auto offsetx = m_conf.get(m_conf.section(), "offset-x", ""s); auto offsety = m_conf.get(m_conf.section(), "offset-y", ""s); - m_opts.size.w = atoi(w.c_str()); - m_opts.size.h = atoi(h.c_str()); - m_opts.offset.x = atoi(offsetx.c_str()); - m_opts.offset.y = atoi(offsety.c_str()); + m_opts.size.w = std::strtol(w.c_str(), nullptr, 10); + m_opts.size.h = std::strtol(h.c_str(), nullptr, 10); + m_opts.offset.x = std::strtol(offsetx.c_str(), nullptr, 10); + m_opts.offset.y = std::strtol(offsety.c_str(), nullptr, 10); // Evaluate percentages double tmp; - if ((tmp = atof(w.c_str())) && w.find('%') != string::npos) { + if ((tmp = strtof(w.c_str(), nullptr)) && w.find('%') != string::npos) { m_opts.size.w = math_util::percentage_to_value(tmp, m_opts.monitor->w); } - if ((tmp = atof(h.c_str())) && h.find('%') != string::npos) { + if ((tmp = strtof(h.c_str(), nullptr)) && h.find('%') != string::npos) { m_opts.size.h = math_util::percentage_to_value(tmp, m_opts.monitor->h); } - if ((tmp = atof(offsetx.c_str())) != 0 && offsetx.find('%') != string::npos) { + if ((tmp = strtof(offsetx.c_str(), nullptr)) != 0 && offsetx.find('%') != string::npos) { m_opts.offset.x = math_util::percentage_to_value(tmp, m_opts.monitor->w); } - if ((tmp = atof(offsety.c_str())) != 0 && offsety.find('%') != string::npos) { + if ((tmp = strtof(offsety.c_str(), nullptr)) != 0 && offsety.find('%') != string::npos) { m_opts.offset.y = math_util::percentage_to_value(tmp, m_opts.monitor->h); } diff --git a/src/components/builder.cpp b/src/components/builder.cpp index 4bab278f..33d972d1 100644 --- a/src/components/builder.cpp +++ b/src/components/builder.cpp @@ -123,7 +123,7 @@ void builder::node(string str, bool add_space) { s.erase(0, 5); } else if ((n = s.find("%{T")) == 0 && (m = s.find('}')) != string::npos) { - font(atoi(s.substr(n + 3, m - 3).c_str())); + font(strtol(s.substr(n + 3, m - 3).c_str(), nullptr, 10)); s.erase(n, m + 1); } else if ((n = s.find("%{U-}")) == 0) { diff --git a/src/components/config.cpp b/src/components/config.cpp index 6b2d7b73..ad31bbfc 100644 --- a/src/components/config.cpp +++ b/src/components/config.cpp @@ -226,12 +226,12 @@ char config::convert(string&& value) const { template <> int config::convert(string&& value) const { - return std::atoi(value.c_str()); + return std::strtol(value.c_str(), nullptr, 10); } template <> short config::convert(string&& value) const { - return static_cast(std::atoi(value.c_str())); + return static_cast(std::strtol(value.c_str(), nullptr, 10)); } template <> diff --git a/src/components/parser.cpp b/src/components/parser.cpp index a7e3cbf3..c5231928 100644 --- a/src/components/parser.cpp +++ b/src/components/parser.cpp @@ -106,7 +106,7 @@ void parser::codeblock(string&& data, const bar_settings& bar) { break; case 'O': - m_sig.emit(offset_pixel{static_cast(std::atoi(value.c_str()))}); + m_sig.emit(offset_pixel{static_cast(std::strtol(value.c_str(), nullptr, 10))}); break; case 'l': diff --git a/src/components/renderer.cpp b/src/components/renderer.cpp index 77017901..2c7840e5 100644 --- a/src/components/renderer.cpp +++ b/src/components/renderer.cpp @@ -153,7 +153,7 @@ renderer::renderer( string pattern{f}; size_t pos = pattern.rfind(';'); if (pos != string::npos) { - offset = std::atoi(pattern.substr(pos + 1).c_str()); + offset = std::strtol(pattern.substr(pos + 1).c_str(), nullptr, 10); pattern.erase(pos); } auto font = cairo::make_font(*m_context, string{pattern}, offset, dpi_x, dpi_y); diff --git a/src/ipc.cpp b/src/ipc.cpp index 438b162e..d084e32e 100644 --- a/src/ipc.cpp +++ b/src/ipc.cpp @@ -69,7 +69,7 @@ int main(int argc, char** argv) { log(E_INVALID_CHANNEL, "No channel available for pid " + args[1]); } - pid = atoi(args[1].c_str()); + pid = strtol(args[1].c_str(), nullptr, 10); args.erase(args.begin()); args.erase(args.begin()); } diff --git a/src/modules/menu.cpp b/src/modules/menu.cpp index f43f4d8e..c351e59b 100644 --- a/src/modules/menu.cpp +++ b/src/modules/menu.cpp @@ -76,7 +76,7 @@ namespace modules { auto spacing = m_formatter->get(get_format())->spacing; for (auto&& item : m_levels[m_level]->items) { /* - * Depending on whether the menu items are to the left or right of the toggle label, the items need to be + * Depending on whether the menu items are to the left or right of the toggle label, the items need to be * drawn before or after the spacings and the separator * * If the menu expands to the left, the separator should be drawn on the right side because otherwise the menu @@ -132,7 +132,7 @@ namespace modules { if (level.empty()) { level = "0"; } - m_level = std::atoi(level.c_str()); + m_level = std::strtol(level.c_str(), nullptr, 10); m_log.info("%s: Opening menu level '%i'", name(), static_cast(m_level)); if (static_cast(m_level) >= m_levels.size()) { diff --git a/src/modules/mpd.cpp b/src/modules/mpd.cpp index 4a5e84a2..08fd1f7e 100644 --- a/src/modules/mpd.cpp +++ b/src/modules/mpd.cpp @@ -368,11 +368,11 @@ namespace modules { if (s.empty()) { return false; } else if (s[0] == '+') { - percentage = status->get_elapsed_percentage() + std::atoi(s.substr(1).c_str()); + percentage = status->get_elapsed_percentage() + std::strtol(s.substr(1).c_str(), nullptr, 10); } else if (s[0] == '-') { - percentage = status->get_elapsed_percentage() - std::atoi(s.substr(1).c_str()); + percentage = status->get_elapsed_percentage() - std::strtol(s.substr(1).c_str(), nullptr, 10); } else { - percentage = std::atoi(s.c_str()); + percentage = std::strtol(s.c_str(), nullptr, 10); } mpd->seek(status->get_songid(), status->get_seek_position(percentage)); } else { diff --git a/src/modules/temperature.cpp b/src/modules/temperature.cpp index a7dde4aa..da27b7c2 100644 --- a/src/modules/temperature.cpp +++ b/src/modules/temperature.cpp @@ -50,7 +50,7 @@ namespace modules { } bool temperature_module::update() { - m_temp = std::atoi(file_util::contents(m_path).c_str()) / 1000.0f + 0.5f; + m_temp = std::strtol(file_util::contents(m_path).c_str(), nullptr, 10) / 1000.0f + 0.5f; int m_temp_f = floor(((1.8 * m_temp) + 32) + 0.5); m_perc = math_util::cap(math_util::percentage(m_temp, 0, m_tempwarn), 0, 100); diff --git a/src/x11/tray_manager.cpp b/src/x11/tray_manager.cpp index 635f4301..5e2cc767 100644 --- a/src/x11/tray_manager.cpp +++ b/src/x11/tray_manager.cpp @@ -144,8 +144,8 @@ void tray_manager::setup(const bar_settings& bar_opts) { auto offset_x_def = conf.get(bs, "tray-offset-x", ""s); auto offset_y_def = conf.get(bs, "tray-offset-y", ""s); - auto offset_x = atoi(offset_x_def.c_str()); - auto offset_y = atoi(offset_y_def.c_str()); + auto offset_x = strtol(offset_x_def.c_str(), nullptr, 10); + auto offset_y = strtol(offset_y_def.c_str(), nullptr, 10); if (offset_x != 0 && offset_x_def.find('%') != string::npos) { if (m_opts.detached) {