Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Panic using local timestamps #84

Open
allenap opened this issue Oct 20, 2023 · 1 comment
Open

Panic using local timestamps #84

allenap opened this issue Oct 20, 2023 · 1 comment
Labels
CVE-2020-26235 Unsound behaviour with local timezone access

Comments

@allenap
Copy link

allenap commented Oct 20, 2023

simple_logger v4.2.0

Using:

    simple_logger::SimpleLogger::new()
        .with_level(log::LevelFilter::Warn)
        .with_local_timestamps()
        .with_colors(stdout().is_terminal())
        .env()
        .init()?;

I get:

The application panicked (crashed).
Message:  Could not determine the UTC offset on this system. Consider displaying UTC time instead. Possible causes are that the time crate does not implement "local_offset_at" on your system, or that you are running in a multi-threaded environment and the time crate is returning "None" from "local_offset_at" to avoid unsafe behaviour. See the time crate's documentation for more information. (https://time-rs.github.io/internal-api/time/index.html#feature-flags): IndeterminateOffset
Location: /Users/gavin/.cargo/registry/src/index.crates.io-6f17d22bba15001f/simple_logger-4.2.0/src/lib.rs:447

This is inside a Tokio block:

    let rt = tokio::runtime::Runtime::new()?;
    rt.block_on(async {
      log::info!("... something ...");
    })

so the error message makes sense – but perhaps a logging library shouldn't panic in this kind of situation.

What would work well for my scenario (i.e. it may not be useful for others):

  • The UTC offset is captured at initialisation time and used as a fallback.
  • If there's no offset at initialisation time, emit a single warning. Log messages should then be emitted in UTC.
  • When the fallback value is used, emit a warning to say that the offset may be inaccurate.

Or:

  • When the offset cannot be determined, logs are emitted in UTC.
  • Additionally, if the log timestamp format doesn't include offset, emit a warning that the timestamp may be inaccurate.
@borntyping
Copy link
Owner

borntyping commented Oct 20, 2023

Hi! 👋

  • The UTC offset is captured at initialisation time and used as a fallback.
  • If there's no offset at initialisation time, emit a single warning. Log messages should then be emitted in UTC.
  • When the fallback value is used, emit a warning to say that the offset may be inaccurate.

You shouldn't do this — offsets can change while a program is running, usually for summer time transitions. It might be nice if we could check if we can get the offset during initialisation, but I'm not sure that would cover your case as the initialisation is likely happening before the threaded code starts.

When the offset cannot be determined, logs are emitted in UTC.

A timestamps option to use UTC or local time might be a nice feature—I don't work on this library much but am usually happy to accept pull requests that keep within the library's simple spirit! However I'm not sure it's particularly ideal to have logs that might change format over time or in different parts of the program. In your example the threaded code would output UTC timestamps, but any log statements executed during the applications setup would use local timestamps.

You can find a bit more about the history of this issue on #52 and the issues linked to it. I'd recommend switching to UTC timestamps in any case where this complexity becomes relevant.

Since I last wrote about this issue time also added an API for changing it's soundness, which will enable getting a UTC offset in threaded code as long as you are absolutely confident the environment will not be mutated at the same time as UTC offsets are obtained. https://time-rs.github.io/internal-api/time/util/local_offset/fn.set_soundness.html

@borntyping borntyping added the CVE-2020-26235 Unsound behaviour with local timezone access label Oct 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CVE-2020-26235 Unsound behaviour with local timezone access
Projects
None yet
Development

No branches or pull requests

2 participants