lingucomponent/source/languageguessing/guess.cxx | 75 +++++++++++------------
1 file changed, 36 insertions(+), 39 deletions(-)
New commits:
commit 31a8d9c711dfdbdb4b11067125d763b1d570033b
Author: Stephan Bergmann <sberg...@redhat.com>
AuthorDate: Fri Oct 26 13:16:57 2018 +0200
Commit: Stephan Bergmann <sberg...@redhat.com>
CommitDate: Fri Oct 26 14:44:56 2018 +0200
Don't read past end of string in Guess ctor
<https://ci.libreoffice.org//job/lo_ubsan/1082/>:
> ==26422==ERROR: AddressSanitizer: heap-buffer-overflow on address
0x604000accc72 at pc 0x2ae43e63f4b6 bp 0x2ae43e600510 sp 0x2ae43e600508
> READ of size 1 at 0x604000accc72 thread T70 (cppu_threadpool)
> #0 0x2ae43e63f4b5 in Guess::Guess(char const*)
/lingucomponent/source/languageguessing/guess.cxx:95:28
> #1 0x2ae43e667f2f in SimpleGuesser::GetManagedLanguages(char)
/lingucomponent/source/languageguessing/simpleguesser.cxx:169:19
> #2 0x2ae43e668420 in SimpleGuesser::GetAvailableLanguages()
/lingucomponent/source/languageguessing/simpleguesser.cxx:179:12
> #3 0x2ae43e64a18e in LangGuess_Impl::getEnabledLanguages()
/lingucomponent/source/languageguessing/guesslang.cxx:229:24
[...]
> 0x604000accc72 is located 0 bytes to the right of 34-byte region
[0x604000accc50,0x604000accc72)
> allocated by thread T70 (cppu_threadpool) here:
[...]
> #7 0x2ae43e65350a in std::string::operator+=(char const*)
/home/tdf/lode/opt_private/lib/gcc/x86_64-unknown-linux-gnu/5.2.0/../../../../include/c++/5.2.0/bits/basic_string.h:3355:16
> #8 0x2ae43e667e6e in SimpleGuesser::GetManagedLanguages(char)
/lingucomponent/source/languageguessing/simpleguesser.cxx:168:21
> #9 0x2ae43e668420 in SimpleGuesser::GetAvailableLanguages()
/lingucomponent/source/languageguessing/simpleguesser.cxx:179:12
> #10 0x2ae43e64a18e in LangGuess_Impl::getEnabledLanguages()
/lingucomponent/source/languageguessing/guesslang.cxx:229:24
[...]
shows, during UITest_librelogo, the Guess ctor making wrong assumptions
about
the structure of guess_str and skipping over the terminating NUL. Locally I
could see that while most inputs do have the expected syntax of starting
with
"[" and containing two "-", one input is indeed just "[haw-utf8" without a
second "-".
I don't know where the strings passed into the Guess ctor in the two places
in
lingucomponent/source/languageguessing/simpleguesser.cxx ultimately come
from,
and what their guaranteed syntax and their semantics is. So from the
existing
code and the non--well-formed "[haw-utf8" sample (where the second segment
shall
apparently designate an encoding, not a country), construct rules how to
robustly parse any input into potential language/country/encoding parts.
(What
is obvious from the call sites is that for one each input will start with
"[",
and for another the item to parse need neither be "]"- nor NUL-terminated.)
(Guess::encoding_str and the local enc variable have effectively been unused
ever since the code's introduction in
0762811922c4e1a3474bdd91daf308baafbc18e4
"INTEGRATION: CWS languageguessing". Guess::encoding_str, but not the local
enc variable, got later removed with
b275246c30ce3796cd22f72cd82c58b5cf4c86f0
"loplugin:unusedfields in formula..registry".)
Change-Id: Icbedc05ed5b119ee4efbc3118cc17076a4d80c74
Reviewed-on: https://gerrit.libreoffice.org/62390
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sberg...@redhat.com>
diff --git a/lingucomponent/source/languageguessing/guess.cxx
b/lingucomponent/source/languageguessing/guess.cxx
index eefa7ab6c0f0..a2f3be35382b 100644
--- a/lingucomponent/source/languageguessing/guess.cxx
+++ b/lingucomponent/source/languageguessing/guess.cxx
@@ -17,6 +17,9 @@
* the License at http://www.apache.org/licenses/LICENSE-2.0 .
*/
+#include <sal/config.h>
+
+#include <cassert>
#include <iostream>
#include <string.h>
@@ -39,11 +42,6 @@
using namespace std;
-static bool isSeparator(const char c){
- return c == GUESS_SEPARATOR_OPEN || c == GUESS_SEPARATOR_SEP || c ==
GUESS_SEPARATOR_CLOSE || c == '\0';
-}
-
-
Guess::Guess()
: language_str(DEFAULT_LANGUAGE)
, country_str(DEFAULT_COUNTRY)
@@ -59,48 +57,47 @@ Guess::Guess(const char * guess_str)
: language_str(DEFAULT_LANGUAGE)
, country_str(DEFAULT_COUNTRY)
{
- string lang;
- string country;
- string enc;
-
//if the guess is not like "UNKNOWN" or "SHORT", go into the brackets
if(strcmp(guess_str + 1, TEXTCAT_RESULT_UNKNOWN_STR) != 0
&&
strcmp(guess_str + 1, TEXTCAT_RESULT_SHORT_STR) != 0)
{
-
- int current_pointer = 0;
-
- //this is to go to the first char of the guess string ( the '[' of
"[en-US-utf8]" )
- while(!isSeparator(guess_str[current_pointer])){
- current_pointer++;
+ // From how this ctor is called from SimpleGuesser::GuessLanguage and
+ // SimpleGuesser::GetManagedLanguages in
+ // lingucomponent/source/languageguessing/simpleguesser.cxx, guess_str
must start with "[":
+ assert(guess_str[0] == GUESS_SEPARATOR_OPEN);
+ auto const start = guess_str + 1;
+ // Only look at the prefix of guess_str, delimited by the next "]" or
"[" or end-of-string;
+ // split it into at most three segments separated by "-" (where excess
occurrences of "-"
+ // would become part of the third segment), like "en-US-utf8"; the
first segment denotes the
+ // language; if there are three segments, the second denotes the
country and the third the
+ // encoding; otherwise, the second segment, if any (e.g., in
"haw-utf8"), denotes the
+ // encoding:
+ char const * dash1 = nullptr;
+ char const * dash2 = nullptr;
+ auto p = start;
+ for (;; ++p) {
+ auto const c = *p;
+ if (c == '\0' || c == GUESS_SEPARATOR_OPEN || c ==
GUESS_SEPARATOR_CLOSE) {
+ break;
+ }
+ if (c == GUESS_SEPARATOR_SEP) {
+ if (dash1 == nullptr) {
+ dash1 = p;
+ } else {
+ dash2 = p;
+ // The encoding is ignored, so we can stop as soon as we
found the second "-":
+ break;
+ }
+ }
}
- current_pointer++;
-
- //this is to pick up the language ( the "en" from "[en-US-utf8]" )
- while(!isSeparator(guess_str[current_pointer])){
- lang+=guess_str[current_pointer];
- current_pointer++;
- }
- current_pointer++;
-
- //this is to pick up the country ( the "US" from "[en-US-utf8]" )
- while(!isSeparator(guess_str[current_pointer])){
- country+=guess_str[current_pointer];
- current_pointer++;
+ auto const langLen = (dash1 == nullptr ? p : dash1) - start;
+ if (langLen != 0) { // if not we use the default value
+ language_str.assign(start, langLen);
}
- current_pointer++;
-
- //this is to pick up the encoding ( the "utf8" from "[en-US-utf8]" )
- while(!isSeparator(guess_str[current_pointer])){
- enc+=guess_str[current_pointer];
- current_pointer++;
- }
-
- if(!lang.empty()){//if not we use the default value
- language_str=lang;
+ if (dash2 != nullptr) {
+ country_str.assign(dash1 + 1, dash2 - (dash1 + 1));
}
- country_str=country;
}
}
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
