Harden file opening and related parsing paths

automation/tests/aegisub.cpp

@@ -60,7 +60,7 @@ int main(int argc, char **argv) {
 	}
 
 	preload_modules(L);
-	Install(L, {"include"});
+	Install(L, {"include"}, "include");
 
 	// Patch os.exit to close the lua state first since busted calls it when
 	// it's done
@@ -93,4 +93,3 @@ int main(int argc, char **argv) {
 	check(L, lua_pcall(L, argc - 2, LUA_MULTRET, base));
 	lua_close(L);
 }
-

libaegisub/common/file_mapping.cpp

@@ -34,19 +34,30 @@ char *map(int64_t s_offset, uint64_t length, boost::interprocess::mode_t mode,
 	std::unique_ptr<mapped_region>& region, uint64_t& mapping_start)
 {
 	static char dummy = 0;
-	if (length == 0) return &dummy;
+	if (s_offset < 0)
+		throw agi::InternalError("Attempted to map a negative file offset");
 
 	auto offset = static_cast<uint64_t>(s_offset);
-	if (offset + length > file_size)
+	if (offset > file_size || length > file_size - offset)
 		throw agi::InternalError("Attempted to map beyond end of file");
+	if (length == 0) return &dummy;
 
 	// Check if we can just use the current mapping
-	if (region && offset >= mapping_start && offset + length <= mapping_start + region->get_size())
-		return static_cast<char *>(region->get_address()) + offset - mapping_start;
+	if (region && offset >= mapping_start) {
+		auto relative_offset = offset - mapping_start;
+		auto region_size = region->get_size();
+		if (relative_offset <= region_size && length <= region_size - relative_offset)
+			return static_cast<char *>(region->get_address()) + relative_offset;
+	}
 
 	if (sizeof(size_t) == 4) {
 		mapping_start = offset & ~0xFFFFFULL; // Align to 1 MB boundary
-		length += static_cast<size_t>(offset - mapping_start);
+		auto prefix = offset - mapping_start;
+		if (length > std::numeric_limits<size_t>::max() - prefix)
+			throw std::bad_alloc();
+		length += prefix;
+		if (length > std::numeric_limits<size_t>::max() - 0xFFFFF)
+			throw std::bad_alloc();
 		// Map 16 MB or length rounded up to the next MB
 		length = std::min<uint64_t>(std::max<uint64_t>(0x1000000U, (length + 0xFFFFF) & ~0xFFFFF), file_size - mapping_start);
 	}

libaegisub/common/vfr.cpp

@@ -21,19 +21,36 @@
 #include "libaegisub/charset.h"
 #include "libaegisub/io.h"
 #include "libaegisub/line_iterator.h"
+#include "libaegisub/util.h"
 
 #include <algorithm>
 #include <boost/interprocess/streams/bufferstream.hpp>
 #include <boost/range/algorithm.hpp>
 #include <cmath>
 #include <functional>
 #include <iterator>
+#include <limits>
 
 namespace {
 static const int64_t default_denominator = 1000000000;
+static const size_t max_timecodes = 10000000;
 using agi::line_iterator;
 using namespace agi::vfr;
 
+int64_t checked_fps_numerator(double fps) {
+	if (!std::isfinite(fps) || fps < 0.)
+		throw InvalidFramerate("FPS must be greater than zero");
+	if (fps > 1000.)
+		throw InvalidFramerate("FPS must not be greater than 1000");
+	return static_cast<int64_t>(fps * default_denominator);
+}
+
+void append_v1_timecode(std::vector<int>& timecodes, double time) {
+	if (!std::isfinite(time) || time < 0 || time > std::numeric_limits<int>::max() - .5)
+		throw InvalidFramerate("V1 timecode exceeds the supported timestamp range");
+	timecodes.push_back(static_cast<int>(time + .5));
+}
+
 /// @brief Verify that timecodes monotonically increase
 /// @param timecodes List of timecodes to check
 void validate_timecodes(std::vector<int> const& timecodes) {
@@ -76,7 +93,9 @@ TimecodeRange v1_parse_line(std::string const& str) {
 		throw InvalidFramerate("Cannot specify frame rate for negative frames.");
 	if (range.end < range.start)
 		throw InvalidFramerate("End frame must be greater than or equal to start frame");
-	if (range.fps <= 0.)
+	if (range.end > static_cast<int>(max_timecodes) - 2)
+		throw InvalidFramerate("V1 timecode range exceeds the 10000000 frame limit");
+	if (!std::isfinite(range.fps) || range.fps <= 0.)
 		throw InvalidFramerate("FPS must be greater than zero");
 	if (range.fps > 1000.)
 		// This is our limitation, not mkvmerge's
@@ -92,8 +111,14 @@ TimecodeRange v1_parse_line(std::string const& str) {
 /// @param[out] last      Unrounded time of the last frame
 /// @return Assumed fps times one million
 int64_t v1_parse(line_iterator<std::string> file, std::string line, std::vector<int> &timecodes, int64_t &last) {
-	double fps = atof(line.substr(7).c_str());
-	if (fps <= 0.) throw InvalidFramerate("Assumed FPS must be greater than zero");
+	auto fps_string = line.substr(7);
+	auto first = fps_string.find_first_not_of(" \t\r\n\f\v");
+	auto last_char = fps_string.find_last_not_of(" \t\r\n\f\v");
+	if (first != std::string::npos)
+		fps_string = fps_string.substr(first, last_char - first + 1);
+	double fps;
+	if (!agi::util::try_parse(fps_string, &fps) || !std::isfinite(fps) || fps <= 0.)
+		throw InvalidFramerate("Assumed FPS must be greater than zero");
 	if (fps > 1000.) throw InvalidFramerate("Assumed FPS must not be greater than 1000");
 
 	std::vector<TimecodeRange> ranges;
@@ -116,27 +141,28 @@ int64_t v1_parse(line_iterator<std::string> file, std::string line, std::vector<
 			throw InvalidFramerate("Override ranges must not overlap");
 		}
 		for (; frame < range.start; ++frame) {
-			timecodes.push_back(int(time + .5));
+			append_v1_timecode(timecodes, time);
 			time += 1000. / fps;
 		}
 		for (; frame <= range.end; ++frame) {
-			timecodes.push_back(int(time + .5));
+			append_v1_timecode(timecodes, time);
 			time += 1000. / range.fps;
 		}
 	}
-	timecodes.push_back(int(time + .5));
-	last = int64_t(time * fps * default_denominator);
+	append_v1_timecode(timecodes, time);
+	auto last_time = time * fps * default_denominator;
+	if (!std::isfinite(last_time) || last_time >= std::ldexp(1.0, 63))
+		throw InvalidFramerate("V1 timecode exceeds the supported timestamp range");
+	last = static_cast<int64_t>(last_time);
 	return int64_t(fps * default_denominator);
 }
 }
 
 namespace agi::vfr {
 Framerate::Framerate(double fps)
 : denominator(default_denominator)
-, numerator(int64_t(fps * denominator))
+, numerator(checked_fps_numerator(fps))
 {
-	if (fps < 0.) throw InvalidFramerate("FPS must be greater than zero");
-	if (fps > 1000.) throw InvalidFramerate("FPS must not be greater than 1000");
 	timecodes.push_back(0);
 }
 
@@ -178,7 +204,11 @@ Framerate::Framerate(agi::fs::path const& filename)
 	auto encoding = agi::charset::Detect(filename);
 	auto line = *line_iterator<std::string>(*file, encoding.c_str());
 	if (line == "# timecode format v2") {
-		copy(line_iterator<int>(*file, encoding.c_str()), line_iterator<int>(), back_inserter(timecodes));
+		for (auto timecode : line_iterator<int>(*file, encoding.c_str())) {
+			if (timecodes.size() == max_timecodes)
+				throw InvalidFramerate("Timecode file exceeds the 10000000 entry limit");
+			timecodes.push_back(timecode);
+		}
 		SetFromTimecodes();
 		return;
 	}

libaegisub/include/libaegisub/line_iterator.h

@@ -109,10 +109,11 @@ line_iterator<T> end(line_iterator<T>&) { return agi::line_iterator<T>(); }
 template<class OutputType>
 void line_iterator<OutputType>::next() {
 	std::string str;
-	if (!getline(str))
-		return;
-	if (!convert(str))
-		next();
+	do {
+		str.clear();
+		if (!getline(str))
+			return;
+	} while (!convert(str));
 }
 
 template<>

libaegisub/include/libaegisub/lua/script_reader.h

@@ -23,6 +23,8 @@ namespace agi::lua {
 	/// Load a Lua or Moonscript file at the given path
 	bool LoadFile(lua_State *L, agi::fs::path const& filename);
 	/// Install our module loader and add include_path to the module search
-	/// path of the given lua state
-	bool Install(lua_State *L, std::vector<agi::fs::path> const& include_path);
+	/// path of the given lua state. Mandatory runtime support is loaded only
+	/// from support_path.
+	bool Install(lua_State *L, std::vector<agi::fs::path> const& include_path,
+		agi::fs::path const& support_path);
 }

libaegisub/lua/script_reader.cpp

@@ -25,6 +25,18 @@
 #include <lauxlib.h>
 
 namespace agi::lua {
+	namespace {
+		bool load_support_file(lua_State *L, fs::path const& filename) {
+			try {
+				return LoadFile(L, filename);
+			}
+			catch (agi::Exception const& e) {
+				lua_pushstring(L, e.GetMessage().c_str());
+				return false;
+			}
+		}
+	}
+
 	bool LoadFile(lua_State *L, agi::fs::path const& raw_filename) {
 		auto filename = raw_filename;
 		try {
@@ -119,7 +131,8 @@ namespace agi::lua {
 		return lua_gettop(L) - pretop;
 	}
 
-	bool Install(lua_State *L, std::vector<fs::path> const& include_path) {
+	bool Install(lua_State *L, std::vector<fs::path> const& include_path,
+		fs::path const& support_path) {
 		// set the module load path to include_path
 		lua_getglobal(L, "package");
 		push_value(L, "path");
@@ -148,18 +161,42 @@ namespace agi::lua {
 
 #ifdef _WIN32
 		// Replace the default lua IO functions with our unicode compatible ones
-		luaL_loadstring(L, "require('unicode-monkeypatch')");
+		if (!load_support_file(L, support_path / "unicode-monkeypatch.lua"))
+			return false;
 		if (lua_pcall(L, 0, 0, 0)) {
 			return false; // leave error message
 		}
 #endif
 
-		luaL_loadstring(L, "return require('moonscript').loadstring");
+		// Load mandatory support by exact path. In particular, do not allow a
+		// moonscript.lua beside an approved subtitle-local script to run first.
+		if (!load_support_file(L, support_path / "moonscript.lua"))
+			return false;
 		if (lua_pcall(L, 0, 1, 0)) {
 			return false; // leave error message
 		}
+		if (!lua_istable(L, -1)) {
+			lua_pop(L, 1);
+			lua_pushliteral(L, "Bundled moonscript.lua did not return a module table");
+			return false;
+		}
+
+		lua_getfield(L, -1, "loadstring");
+		if (!lua_isfunction(L, -1)) {
+			lua_pop(L, 2);
+			lua_pushliteral(L, "Bundled moonscript.lua has no loadstring function");
+			return false;
+		}
 		lua_setfield(L, LUA_REGISTRYINDEX, "moonscript");
 
+		// Direct loading bypasses require(), so record the trusted module as
+		// loaded before user code gets control.
+		lua_getglobal(L, "package");
+		lua_getfield(L, -1, "loaded");
+		lua_pushvalue(L, -3);
+		lua_setfield(L, -2, "moonscript");
+		lua_pop(L, 3); // loaded, package, module
+
 		return true;
 	}
 }

src/ass_attachment.cpp

@@ -23,6 +23,8 @@
 
 #include <boost/algorithm/string/predicate.hpp>
 
+#include <utility>
+
 // Out-of-line to anchor vtable
 AssEntryGroup AssAttachment::Group() const { return group; }
 
@@ -48,15 +50,23 @@ AssAttachment::AssAttachment(agi::fs::path const& name, AssEntryGroup group)
 		agi::ass::UUEncode(buff, buff + file.size()));
 }
 
+void AssAttachment::SetEntryData(std::string data) {
+	entry_data = std::move(data);
+}
+
 size_t AssAttachment::GetSize() const {
-	auto header_end = entry_data.get().find('\n');
-	return entry_data.get().size() - header_end - 1;
+	auto const& data = entry_data.get();
+	auto header_end = data.find('\n');
+	return data.size() - header_end - 1;
 }
 
 void AssAttachment::Extract(agi::fs::path const& filename) const {
-	auto header_end = entry_data.get().find('\n');
-	auto decoded = agi::ass::UUDecode(entry_data.get().c_str() + header_end + 1, &entry_data.get().back() + 1);
-	agi::io::Save(filename, true).Get().write(&decoded[0], decoded.size());
+	auto const& data = entry_data.get();
+	auto header_end = data.find('\n');
+	auto decoded = agi::ass::UUDecode(data.c_str() + header_end + 1, &data.back() + 1);
+	agi::io::Save save(filename, true);
+	if (!decoded.empty())
+		save.Get().write(decoded.data(), decoded.size());
 }
 
 std::string AssAttachment::GetFileName(bool raw) const {

src/ass_attachment.h

@@ -33,8 +33,8 @@ class AssAttachment final : public AssEntry {
 	/// Get the size of the attached file in bytes
 	size_t GetSize() const;
 
-	/// Add a line of data (without newline) read from a subtitle file
-	void AddData(std::string const& data) { entry_data = entry_data.get() + data + "\r\n"; }
+	/// Store entry data accumulated by the subtitle parser.
+	void SetEntryData(std::string data);
 
 	/// Extract the contents of this attachment to a file
 	/// @param filename Path to save the attachment to

src/ass_override.cpp

@@ -362,7 +362,12 @@ void parse_parameters(AssOverrideTag *tag, std::string_view text, AssOverrideTag
 	std::vector<std::string> paramList = tokenize(text);
 	size_t totalPars = paramList.size();
 
-	int parsFlag = 1 << (totalPars - 1); // Get optional parameters flag
+	// Optional-parameter masks only describe the supported 1-8 parameter
+	// forms. Avoid shifting by a negative or oversized count for malformed
+	// tags with no parameters or far too many of them.
+	unsigned parsFlag = totalPars > 0 && totalPars <= 8
+		? 1u << (totalPars - 1)
+		: 0;
 	// vector (i)clip is the second clip proto_ittype in the list
 	if ((tag->Name == "\\clip" || tag->Name == "\\iclip") && totalPars != 4) {
 		++proto_it;

src/ass_parser.cpp

@@ -103,6 +103,12 @@ AssParser::AssParser(AssFile *target, int version)
 
 AssParser::~AssParser() = default;
 
+void AssParser::FinishAttachment() {
+	attach->SetEntryData(std::move(attachment_data));
+	target->Attachments.push_back(std::move(*attach));
+	attach.reset();
+}
+
 void AssParser::ParseAttachmentLine(std::string const& data) {
 	bool is_filename = data.starts_with("fontname: ") || data.starts_with("filename: ");
 
@@ -116,19 +122,21 @@ void AssParser::ParseAttachmentLine(std::string const& data) {
 
 	// Data is over, add attachment to the file
 	if (!valid_data || is_filename) {
-		target->Attachments.push_back(*attach.release());
+		FinishAttachment();
 		AddLine(data);
 	}
 	else {
-		attach->AddData(data);
+		attachment_data.append(data).append("\r\n");
 
 		// Done building
 		if (data.size() < 80)
-			target->Attachments.push_back(*attach.release());
+			FinishAttachment();
 	}
 }
 
-void AssParser::ParseScriptInfoLine(std::string const& data) {
+void AssParser::ParseScriptInfoLine(std::string const& rawdata) {
+	std::string data = SanitizeLine(rawdata);
+
 	if (data.starts_with(";")) {
 		// Skip stupid comments added by other programs
 		// Of course, we'll add our own in place later... ;)
@@ -187,13 +195,17 @@ void AssParser::ParseStyleLine(std::string const& data) {
 }
 
 void AssParser::ParseFontLine(std::string const& data) {
-	if (data.starts_with("fontname: "))
+	if (data.starts_with("fontname: ")) {
 		attach = std::make_unique<AssAttachment>(data, AssEntryGroup::FONT);
+		attachment_data = data + "\r\n";
+	}
 }
 
 void AssParser::ParseGraphicsLine(std::string const& data) {
-	if (data.starts_with("filename: "))
+	if (data.starts_with("filename: ")) {
 		attach = std::make_unique<AssAttachment>(data, AssEntryGroup::GRAPHIC);
+		attachment_data = data + "\r\n";
+	}
 }
 
 void AssParser::ParseExtradataLine(std::string const &rawdata) {

src/ass_parser.h

@@ -25,8 +25,10 @@ class AssParser {
 	AssFile *target;
 	int version;
 	std::unique_ptr<AssAttachment> attach;
+	std::string attachment_data;
 	void (AssParser::*state)(std::string const&);
 
+	void FinishAttachment();
 	void ParseAttachmentLine(std::string const& data);
 	void ParseEventLine(std::string const& data);
 	void ParseStyleLine(std::string const& data);