1758080 - Empty user.js causes a panic in mozprofile

Status: RESOLVED FIXED

(Testing :: Mozbase Rust, defect)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Originally filed as https://github.com/mozilla/geckodriver/issues/1990

Geckodriver panics when trying to read an empty user.js file which could be part of a custom profile:

thread 'webdriver dispatcher' panicked at 'attempt to subtract with overflow', testing/mozbase/rust/mozprofile/src/prefreader.rs:262:24
stack backtrace:
   0: rust_begin_unwind
             at /rustc/db9d1b20bba1968c1ec1fc49616d4742c1725b4b/library/std/src/panicking.rs:498:5
   1: core::panicking::panic_fmt
             at /rustc/db9d1b20bba1968c1ec1fc49616d4742c1725b4b/library/core/src/panicking.rs:107:14
   2: core::panicking::panic
             at /rustc/db9d1b20bba1968c1ec1fc49616d4742c1725b4b/library/core/src/panicking.rs:48:5
   3: mozprofile::prefreader::PrefTokenizer::get_char
             at ./testing/mozbase/rust/mozprofile/src/prefreader.rs:262:24
   4: mozprofile::prefreader::PrefTokenizer::next_token
             at ./testing/mozbase/rust/mozprofile/src/prefreader.rs:442:31
   5: <mozprofile::prefreader::PrefTokenizer as core::iter::traits::iterator::Iterator>::next
             at ./testing/mozbase/rust/mozprofile/src/prefreader.rs:746:32
   6: mozprofile::prefreader::parse_tokens
             at ./testing/mozbase/rust/mozprofile/src/prefreader.rs:883:19
   7: mozprofile::prefreader::parse
             at ./testing/mozbase/rust/mozprofile/src/prefreader.rs:1051:5
   8: mozprofile::profile::PrefFile::new
             at ./testing/mozbase/rust/mozprofile/src/profile.rs:81:13
   9: mozprofile::profile::Profile::user_prefs
             at ./testing/mozbase/rust/mozprofile/src/profile.rs:60:36
  10: geckodriver::browser::set_prefs
             at ./testing/geckodriver/src/browser.rs:269:17
  11: geckodriver::browser::LocalBrowser::new
             at ./testing/geckodriver/src/browser.rs:81:28
  12: geckodriver::marionette::MarionetteHandler::create_connection
             at ./testing/geckodriver/src/marionette.rs:194:28
  13: <geckodriver::marionette::MarionetteHandler as webdriver::server::WebDriverHandler<geckodriver::command::GeckoExtensionRoute>>::handle_command
             at ./testing/geckodriver/src/marionette.rs:250:36
  14: webdriver::server::Dispatcher<T,U>::run
             at ./testing/webdriver/src/server.rs:84:34
  15: webdriver::server::start::{{closure}}
             at ./testing/webdriver/src/server.rs:243:9

Comment 1

[James Graham [:jgraham]]

Attachment: Bug 1758080 - Fix some overflow bugs in mozprofile,

We had a couple of cases of trying to subtract from unsigned 0, which
panics in a debug buid and overflows in a release build, typically
causing a later read to fail its bounds check.

Comment 2

[James Graham [:jgraham]]

Attachment: Bug 1758080 - Fix handling of invalid integer literal,

We were creating a TokenType::Int for the data "-", which on its own
isn't enough to parse as a valid integer. The implest solution is to
return an error token when interpreting the data as an integer
fails.

This also changes the error token to own its string. Although that
requires some extra copying, hitting an error is by definition
exceptional, and this allows us to write more specific error messages.

Comment 3

[James Graham [:jgraham]]

Attachment: Bug 1758080 - Fix parsing boolean followed by EOF,

In this case we were ungetting the last character in the boolean
rather than the (non-existant) following seperator character, so we
ended up trying to construct a boolean with a string like "tru".

Comment 4

[James Graham [:jgraham]]

Attachment: Bug 1758080 - Add fuzzer script to repo,

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3ef1ccf08394
Fix some overflow bugs in mozprofile, r=webdriver-reviewers,whimboo
https://hg.mozilla.org/integration/autoland/rev/8886085d71e3
Fix handling of invalid integer literal, r=webdriver-reviewers,whimboo
https://hg.mozilla.org/integration/autoland/rev/01e9ad8d7ec1
Fix parsing boolean followed by EOF, r=webdriver-reviewers,whimboo
https://hg.mozilla.org/integration/autoland/rev/862c6ac3df8d
Add fuzzer script to repo, r=webdriver-reviewers,whimboo

Comment 6

[James Graham [:jgraham]]

Attachment: Bug 1758080 - Fix some overflow bugs in mozprofile,

We had a couple of cases of trying to subtract from unsigned 0, which
panics in a debug buid and overflows in a release build, typically
causing a later read to fail its bounds check.

Comment 7

[James Graham [:jgraham]]

Attachment: Bug 1758080 - Fix handling of invalid integer literal,

We were creating a TokenType::Int for the data "-", which on its own
isn't enough to parse as a valid integer. The implest solution is to
return an error token when interpreting the data as an integer
fails.

This also changes the error token to own its string. Although that
requires some extra copying, hitting an error is by definition
exceptional, and this allows us to write more specific error messages.

Comment 8

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/3ef1ccf08394
https://hg.mozilla.org/mozilla-central/rev/8886085d71e3
https://hg.mozilla.org/mozilla-central/rev/01e9ad8d7ec1
https://hg.mozilla.org/mozilla-central/rev/862c6ac3df8d