From 8b00ce7b7dd1ecf1db40a31a995480baf87a08e1 Mon Sep 17 00:00:00 2001 From: Lauris Kaplinski Date: Wed, 17 Dec 2025 22:33:22 +0200 Subject: [PATCH 1/5] Check XML text for unsupported chars and validate utf8 Signed-off-by: Lauris Kaplinski --- src/XMLDocument.h | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/XMLDocument.h b/src/XMLDocument.h index 6c046149d..6ef1ed1be 100644 --- a/src/XMLDocument.h +++ b/src/XMLDocument.h @@ -291,10 +291,21 @@ struct XMLNode: public XMLElem return from_base64(operator sv()); } - XMLNode& operator=(sv text) noexcept + XMLNode& operator=(sv text) { if(!d) return *this; + const char *utf = text.cbegin(); + int len = int(text.size()); + while (utf < text.cend()) { + int uc = xmlGetUTF8Char((const unsigned char *) utf, &len); + if (len < 1) { + THROW("Invalid utf8 string"); + } else if ((uc < 0x20) && (uc != 0x9) && (uc != 0xa) && (uc != 0xd)) { + THROW("Invalid character"); + } + utf += len; + } xmlNodeSetContentLen(d, nullptr, 0); if(!text.empty()) xmlNodeAddContentLen(d, pcxmlChar(text.data()), int(text.length())); From 3e16b0ff0616f450c34dc02e1f945f44c02ccbaa Mon Sep 17 00:00:00 2001 From: Lauris Kaplinski Date: Wed, 17 Dec 2025 22:59:16 +0200 Subject: [PATCH 2/5] Added code comments about utf8/xml requirement Signed-off-by: Lauris Kaplinski --- src/crypto/Signer.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/crypto/Signer.cpp b/src/crypto/Signer.cpp index ea40f8f7c..ca4426e23 100644 --- a/src/crypto/Signer.cpp +++ b/src/crypto/Signer.cpp @@ -73,6 +73,7 @@ Signer::~Signer() = default; * @param stateOrProvince * @param postalCode * @param countryName + * The strings have to be utf8 encoded and not contain any control values (any char < 0x20 except 0x9, 0xa and 0xd) */ void Signer::setSignatureProductionPlace(const string &city, const string &stateOrProvince, const string &postalCode, const string &countryName) @@ -91,6 +92,7 @@ void Signer::setSignatureProductionPlace(const string &city, * @param stateOrProvince * @param postalCode * @param countryName + * The strings have to be utf8 encoded and not contain any control values (any char < 0x20 except 0x9, 0xa and 0xd) */ void Signer::setSignatureProductionPlaceV2(const string &city, const string &streetAddress, const string &stateOrProvince, const string &postalCode, const string &countryName) @@ -197,6 +199,7 @@ void Signer::setProfile(const string &profile) /** * Sets signature roles according XAdES standard. The parameter may contain the signer’s role and optionally the signer’s resolution. Note that only one signer role value (i.e. one <ClaimedRole> XML element) should be used. * If the signer role contains both role and resolution then they must be separated with a slash mark, e.g. “role / resolution”. + * The strings have to be utf8 encoded and not contain any control values (any char < 0x20 except 0x9, 0xa and 0xd) */ void Signer::setSignerRoles(const vector &signerRoles) { From 4b16e67777d84c66ef75c3f1eb8b782bbf400c90 Mon Sep 17 00:00:00 2001 From: Lauris Kaplinski Date: Mon, 5 Jan 2026 11:07:02 +0200 Subject: [PATCH 3/5] Clearer XML charset error message, removed noexcept from setSignerRoles Signed-off-by: Lauris Kaplinski --- src/SignatureXAdES_B.cpp | 2 +- src/SignatureXAdES_B.h | 2 +- src/XMLDocument.h | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/SignatureXAdES_B.cpp b/src/SignatureXAdES_B.cpp index ace6ac43d..d947cce58 100644 --- a/src/SignatureXAdES_B.cpp +++ b/src/SignatureXAdES_B.cpp @@ -746,7 +746,7 @@ void SignatureXAdES_B::setSignatureProductionPlace(string_view name, * * @param roles signer roles. */ -void SignatureXAdES_B::setSignerRoles(string_view name, const vector &roles) noexcept +void SignatureXAdES_B::setSignerRoles(string_view name, const vector &roles) { if(roles.empty()) return; diff --git a/src/SignatureXAdES_B.h b/src/SignatureXAdES_B.h index 56f12e485..aa212fdf7 100644 --- a/src/SignatureXAdES_B.h +++ b/src/SignatureXAdES_B.h @@ -112,7 +112,7 @@ namespace digidoc void setSignatureProductionPlace(std::string_view name, const std::string &city, const std::string &streetAddress, const std::string &stateOrProvince, const std::string &postalCode, const std::string &countryName) noexcept; - void setSignerRoles(std::string_view name, const std::vector &signerRoles) noexcept; + void setSignerRoles(std::string_view name, const std::vector &signerRoles); constexpr XMLNode V1orV2(std::string_view v1, std::string_view v2) const noexcept; // offline checks diff --git a/src/XMLDocument.h b/src/XMLDocument.h index 6ef1ed1be..5df779910 100644 --- a/src/XMLDocument.h +++ b/src/XMLDocument.h @@ -300,9 +300,9 @@ struct XMLNode: public XMLElem while (utf < text.cend()) { int uc = xmlGetUTF8Char((const unsigned char *) utf, &len); if (len < 1) { - THROW("Invalid utf8 string"); - } else if ((uc < 0x20) && (uc != 0x9) && (uc != 0xa) && (uc != 0xd)) { - THROW("Invalid character"); + THROW("Invalid utf8 string in XML content"); + } else if (((uc < 0x20) && (uc != 0x9) && (uc != 0xa) && (uc != 0xd)) || (uc == 0xfffe) || (uc == 0xffff)) { + THROW("Invalid character '0x%2x' in XML content", uc); } utf += len; } From 7f514c6bf6aefb9cc7411196185379860fd9f234 Mon Sep 17 00:00:00 2001 From: Raul Metsma Date: Wed, 1 Apr 2026 09:30:10 +0300 Subject: [PATCH 4/5] Fix build on windows Signed-off-by: Raul Metsma --- src/XMLDocument.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/XMLDocument.h b/src/XMLDocument.h index 5df779910..1ee401401 100644 --- a/src/XMLDocument.h +++ b/src/XMLDocument.h @@ -295,9 +295,10 @@ struct XMLNode: public XMLElem { if(!d) return *this; - const char *utf = text.cbegin(); - int len = int(text.size()); - while (utf < text.cend()) { + const char *utf = std::to_address(text.cbegin()); + const char *end = utf + text.size(); + while (utf < end) { + int len = int(end - utf); // remaining bytes int uc = xmlGetUTF8Char((const unsigned char *) utf, &len); if (len < 1) { THROW("Invalid utf8 string in XML content"); From 6bb2d96f59296a0b10c5043552605f63b884c51e Mon Sep 17 00:00:00 2001 From: Raul Metsma Date: Thu, 16 Jul 2026 11:39:51 +0300 Subject: [PATCH 5/5] Handle spaces in path IB-8215 Signed-off-by: Raul Metsma --- src/SignatureXAdES_B.cpp | 2 +- src/SignatureXAdES_B.h | 2 +- src/XMLDocument.h | 14 +---- src/crypto/Signer.cpp | 35 +++++++++++- src/util/XMLText.h | 99 +++++++++++++++++++++++++++++++++ test/libdigidocpp_boost.cpp | 107 ++++++++++++++++++++++++++++++++++++ 6 files changed, 241 insertions(+), 18 deletions(-) create mode 100644 src/util/XMLText.h diff --git a/src/SignatureXAdES_B.cpp b/src/SignatureXAdES_B.cpp index d947cce58..ace6ac43d 100644 --- a/src/SignatureXAdES_B.cpp +++ b/src/SignatureXAdES_B.cpp @@ -746,7 +746,7 @@ void SignatureXAdES_B::setSignatureProductionPlace(string_view name, * * @param roles signer roles. */ -void SignatureXAdES_B::setSignerRoles(string_view name, const vector &roles) +void SignatureXAdES_B::setSignerRoles(string_view name, const vector &roles) noexcept { if(roles.empty()) return; diff --git a/src/SignatureXAdES_B.h b/src/SignatureXAdES_B.h index aa212fdf7..56f12e485 100644 --- a/src/SignatureXAdES_B.h +++ b/src/SignatureXAdES_B.h @@ -112,7 +112,7 @@ namespace digidoc void setSignatureProductionPlace(std::string_view name, const std::string &city, const std::string &streetAddress, const std::string &stateOrProvince, const std::string &postalCode, const std::string &countryName) noexcept; - void setSignerRoles(std::string_view name, const std::vector &signerRoles); + void setSignerRoles(std::string_view name, const std::vector &signerRoles) noexcept; constexpr XMLNode V1orV2(std::string_view v1, std::string_view v2) const noexcept; // offline checks diff --git a/src/XMLDocument.h b/src/XMLDocument.h index 1ee401401..6c046149d 100644 --- a/src/XMLDocument.h +++ b/src/XMLDocument.h @@ -291,22 +291,10 @@ struct XMLNode: public XMLElem return from_base64(operator sv()); } - XMLNode& operator=(sv text) + XMLNode& operator=(sv text) noexcept { if(!d) return *this; - const char *utf = std::to_address(text.cbegin()); - const char *end = utf + text.size(); - while (utf < end) { - int len = int(end - utf); // remaining bytes - int uc = xmlGetUTF8Char((const unsigned char *) utf, &len); - if (len < 1) { - THROW("Invalid utf8 string in XML content"); - } else if (((uc < 0x20) && (uc != 0x9) && (uc != 0xa) && (uc != 0xd)) || (uc == 0xfffe) || (uc == 0xffff)) { - THROW("Invalid character '0x%2x' in XML content", uc); - } - utf += len; - } xmlNodeSetContentLen(d, nullptr, 0); if(!text.empty()) xmlNodeAddContentLen(d, pcxmlChar(text.data()), int(text.length())); diff --git a/src/crypto/Signer.cpp b/src/crypto/Signer.cpp index ca4426e23..79a5fd3a1 100644 --- a/src/crypto/Signer.cpp +++ b/src/crypto/Signer.cpp @@ -24,6 +24,7 @@ #include "Conf.h" #include "crypto/Digest.h" #include "crypto/X509Cert.h" +#include "util/XMLText.h" #include "util/log.h" #include @@ -35,6 +36,17 @@ using namespace digidoc; using namespace std; +static void requireXMLText(string_view value, string_view field) +{ + const util::XMLTextError error = util::validateXMLText(value); + if(!error) + return; + if(error.reason == util::XMLTextError::Reason::InvalidUTF8) + THROW("Invalid UTF-8 at byte offset %zu in %.*s", error.offset, STR_VIEW_FMT(field)); + THROW("XML 1.0 character U+%04X at byte offset %zu is not allowed in %.*s", + unsigned(error.codePoint), error.offset, STR_VIEW_FMT(field)); +} + class Signer::Private { public: @@ -73,11 +85,17 @@ Signer::~Signer() = default; * @param stateOrProvince * @param postalCode * @param countryName - * The strings have to be utf8 encoded and not contain any control values (any char < 0x20 except 0x9, 0xa and 0xd) + * The strings must be UTF-8 encoded and contain only XML 1.0 characters. + * @throws Exception if a string is not valid UTF-8 or contains a character disallowed by XML 1.0. */ void Signer::setSignatureProductionPlace(const string &city, const string &stateOrProvince, const string &postalCode, const string &countryName) { + requireXMLText(city, "signature production city"); + requireXMLText(stateOrProvince, "signature production state or province"); + requireXMLText(postalCode, "signature production postal code"); + requireXMLText(countryName, "signature production country name"); + d->city = city; d->stateOrProvince = stateOrProvince; d->postalCode = postalCode; @@ -92,11 +110,18 @@ void Signer::setSignatureProductionPlace(const string &city, * @param stateOrProvince * @param postalCode * @param countryName - * The strings have to be utf8 encoded and not contain any control values (any char < 0x20 except 0x9, 0xa and 0xd) + * The strings must be UTF-8 encoded and contain only XML 1.0 characters. + * @throws Exception if a string is not valid UTF-8 or contains a character disallowed by XML 1.0. */ void Signer::setSignatureProductionPlaceV2(const string &city, const string &streetAddress, const string &stateOrProvince, const string &postalCode, const string &countryName) { + requireXMLText(city, "signature production city"); + requireXMLText(streetAddress, "signature production street address"); + requireXMLText(stateOrProvince, "signature production state or province"); + requireXMLText(postalCode, "signature production postal code"); + requireXMLText(countryName, "signature production country name"); + if(!streetAddress.empty()) setENProfile(true); d->city = city; @@ -199,10 +224,14 @@ void Signer::setProfile(const string &profile) /** * Sets signature roles according XAdES standard. The parameter may contain the signer’s role and optionally the signer’s resolution. Note that only one signer role value (i.e. one <ClaimedRole> XML element) should be used. * If the signer role contains both role and resolution then they must be separated with a slash mark, e.g. “role / resolution”. - * The strings have to be utf8 encoded and not contain any control values (any char < 0x20 except 0x9, 0xa and 0xd) + * The strings must be UTF-8 encoded and contain only XML 1.0 characters. + * @throws Exception if a string is not valid UTF-8 or contains a character disallowed by XML 1.0. */ void Signer::setSignerRoles(const vector &signerRoles) { + for(const string &role: signerRoles) + requireXMLText(role, "signer role"); + d->signerRoles = signerRoles; } diff --git a/src/util/XMLText.h b/src/util/XMLText.h new file mode 100644 index 000000000..747103dce --- /dev/null +++ b/src/util/XMLText.h @@ -0,0 +1,99 @@ +/* + * libdigidocpp + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + * + */ + +#pragma once + +#include +#include +#include + +namespace digidoc::util +{ +struct XMLTextError +{ + enum class Reason + { + None, + InvalidUTF8, + InvalidXMLCharacter + }; + + constexpr explicit operator bool() const noexcept { return reason != Reason::None; } + + Reason reason {Reason::None}; + size_t offset {}; + uint32_t codePoint {}; +}; + +inline XMLTextError validateXMLText(std::string_view text) noexcept +{ + for(size_t offset = 0; offset < text.size();) + { + const auto lead = static_cast(text[offset]); + uint32_t codePoint {}; + size_t length {}; + uint32_t minimum {}; + if(lead <= 0x7F) + { + codePoint = lead; + length = 1; + } + else if(lead >= 0xC2 && lead <= 0xDF) + { + codePoint = lead & 0x1F; + length = 2; + minimum = 0x80; + } + else if(lead >= 0xE0 && lead <= 0xEF) + { + codePoint = lead & 0x0F; + length = 3; + minimum = 0x800; + } + else if(lead >= 0xF0 && lead <= 0xF4) + { + codePoint = lead & 0x07; + length = 4; + minimum = 0x10000; + } + else + { + return {XMLTextError::Reason::InvalidUTF8, offset}; + } + + if(length > text.size() - offset) + return {XMLTextError::Reason::InvalidUTF8, offset}; + for(size_t i = 1; i < length; ++i) + { + const auto continuation = static_cast(text[offset + i]); + if((continuation & 0xC0) != 0x80) + return {XMLTextError::Reason::InvalidUTF8, offset}; + codePoint = (codePoint << 6) | (continuation & 0x3F); + } + if(codePoint < minimum || codePoint > 0x10FFFF || + (codePoint >= 0xD800 && codePoint <= 0xDFFF)) + return {XMLTextError::Reason::InvalidUTF8, offset}; + if(codePoint != 0x09 && codePoint != 0x0A && codePoint != 0x0D && + (codePoint < 0x20 || codePoint == 0xFFFE || codePoint == 0xFFFF)) + return {XMLTextError::Reason::InvalidXMLCharacter, offset, codePoint}; + offset += length; + } + return {}; +} +} diff --git a/test/libdigidocpp_boost.cpp b/test/libdigidocpp_boost.cpp index 5a1327fce..1bf83b9f6 100644 --- a/test/libdigidocpp_boost.cpp +++ b/test/libdigidocpp_boost.cpp @@ -30,6 +30,7 @@ #include #include #include +#include namespace digidoc { @@ -115,6 +116,30 @@ BOOST_AUTO_TEST_CASE(signerParameters) }; BOOST_CHECK_EQUAL(signature, sig); } + +BOOST_AUTO_TEST_CASE(signerXMLTextValidation) +{ + PKCS12Signer signer{"signer1.p12", "signer1"}; + signer.setSignerRoles({"Role1"}); + signer.setSignatureProductionPlaceV2("Tartu", "Street", "Tartumaa", "12345", "Estonia"); + + const string invalidControl{"Role\x1B", 5}; + BOOST_CHECK_THROW(signer.setSignerRoles({invalidControl}), Exception); + BOOST_CHECK_EQUAL(signer.signerRoles(), vector{"Role1"}); + BOOST_CHECK_THROW(signer.setSignatureProductionPlace( + invalidControl, "Tartumaa", "12345", "Estonia"), Exception); + BOOST_CHECK_EQUAL(signer.city(), "Tartu"); + + const string invalidUTF8{"Street\xED\xA0\x80", 9}; + BOOST_CHECK_THROW(signer.setSignatureProductionPlaceV2( + "Tartu", invalidUTF8, "Tartumaa", "12345", "Estonia"), Exception); + BOOST_CHECK_EQUAL(signer.streetAddress(), "Street"); + + // Signer metadata is raw UTF-8. XML-looking input is literal text and will + // be escaped by libxml2 when it is eventually inserted into the document. + BOOST_CHECK_NO_THROW(signer.setSignerRoles({""})); + BOOST_CHECK_EQUAL(signer.signerRoles(), vector{""}); +} BOOST_AUTO_TEST_SUITE_END() BOOST_AUTO_TEST_SUITE(X509CertSuite) @@ -706,6 +731,88 @@ BOOST_AUTO_TEST_CASE(WrapContainerWithExpiredSignatures) BOOST_AUTO_TEST_SUITE_END() BOOST_AUTO_TEST_SUITE(XMLTestSuite) +BOOST_AUTO_TEST_CASE(XMLTextValidation) +{ + const auto invalidCharacter = util::validateXMLText(string{"A\x1B" "B", 3}); + BOOST_REQUIRE(invalidCharacter); + BOOST_CHECK(invalidCharacter.reason == util::XMLTextError::Reason::InvalidXMLCharacter); + BOOST_CHECK_EQUAL(invalidCharacter.offset, 1U); + BOOST_CHECK_EQUAL(invalidCharacter.codePoint, 0x1BU); + + for(unsigned char value = 0; value < 0x20; ++value) + { + const auto error = util::validateXMLText(string{char(value)}); + if(value == 0x09 || value == 0x0A || value == 0x0D) + BOOST_CHECK(!error); + else + BOOST_CHECK(error.reason == util::XMLTextError::Reason::InvalidXMLCharacter); + } + + const string invalidUTF8{"A\xED\xA0\x80" "B", 5}; // UTF-8 encoded surrogate U+D800 + const auto invalidEncoding = util::validateXMLText(invalidUTF8); + BOOST_REQUIRE(invalidEncoding); + BOOST_CHECK(invalidEncoding.reason == util::XMLTextError::Reason::InvalidUTF8); + BOOST_CHECK_EQUAL(invalidEncoding.offset, 1U); + + const string legal = "\t\n\r\x7F" + "\xED\x9F\xBF" // U+D7FF + "\xEE\x80\x80" // U+E000 + "\xEF\xBF\xBD" // U+FFFD + "\xF0\x90\x80\x80" // U+10000 + "\xF4\x8F\xBF\xBF"; // U+10FFFF + BOOST_CHECK(!util::validateXMLText(legal)); + + for(string_view value: { + string_view{"\0", 1}, + string_view{"\xC0\x80", 2}, // Overlong U+0000 + string_view{"\x80", 1}, // Isolated continuation byte + string_view{"\xE2\x82", 2}, // Truncated sequence + string_view{"\xF4\x90\x80\x80", 4}, // Above U+10FFFF + string_view{"\xEF\xBF\xBE", 3}, // U+FFFE + string_view{"\xEF\xBF\xBF", 3} // U+FFFF + }) + BOOST_CHECK(util::validateXMLText(value)); +} + +BOOST_AUTO_TEST_CASE(XMLTextEscapingAndParsing) +{ + const string raw = "Jüri & Mari \"'"; + auto doc = XMLDocument::create("root"); + XMLNode root = doc; + BOOST_CHECK_NO_THROW(root = raw); + + string serialized; + BOOST_REQUIRE(doc.save([&serialized](const char *data, size_t size) { + serialized.append(data, size); + })); + BOOST_CHECK(serialized.contains("&")); + BOOST_CHECK(serialized.contains("<Manager>")); + + istringstream input{serialized}; + auto parsed = XMLDocument::openStream(input, {"root"}); + BOOST_CHECK_EQUAL(string(string_view(parsed)), raw); + + // The API accepts raw UTF-8, not pre-escaped XML. A character-reference-like + // string is escaped as literal text and cannot smuggle an illegal character. + root = ""; + serialized.clear(); + BOOST_REQUIRE(doc.save([&serialized](const char *data, size_t size) { + serialized.append(data, size); + })); + BOOST_CHECK(serialized.contains("&#x1B;")); + + istringstream encodedInput{serialized}; + parsed = XMLDocument::openStream(encodedInput, {"root"}); + BOOST_CHECK_EQUAL(string(string_view(parsed)), ""); + + istringstream legalReference{"ä"}; + parsed = XMLDocument::openStream(legalReference, {"root"}); + BOOST_CHECK_EQUAL(string(string_view(parsed)), "ä"); + + istringstream illegalReference{""}; + BOOST_CHECK_THROW(XMLDocument::openStream(illegalReference, {"root"}), Exception); +} + BOOST_AUTO_TEST_CASE(XMLBomb) { BOOST_CHECK_THROW(XMLDocument("xml-bomb-attr.xml"), Exception);