houseabsolute / DateTime-Locale

Localization support for DateTime.pm
https://metacpan.org/release/DateTime-Locale/
Other
3 stars 12 forks source link

Accepted tainted args #30

Closed JRaspass closed 2 years ago

JRaspass commented 2 years ago

At $work we're in the process of upgrading Debian LTS (Stretch → Bullseye). This has caused some fallout in loading locales using tainted variables. This appears to be unintended fallout from the 1.11 change where locales are now read from disk on-demand.

Stretch:

$ perl -MDateTime::Locale -TE 'say DateTime::Locale->VERSION; say DateTime::Locale->load(@ARGV)' en-GB
1.11
DateTime::Locale::FromData=HASH(0x563886777b70)

Bullseye:

$ perl -MDateTime::Locale -TE 'say DateTime::Locale->VERSION; say DateTime::Locale->load(@ARGV)' en-GB
1.31
Insecure dependency in do while running with -T switch at /usr/share/perl5/DateTime/Locale/Data.pm line 6086.

Assuming the change was unintentional this commit restores the old behaviour while keeping the data on disk. I don't think it's unreasonable that a locale code would be tainted if it came from the database of the web request.

The commit does a little validation but to be honest it's probably overkill as the value is checked higher up the stack, so we could easily make the regex /(.+)/ if you prefer.

The test should be skipping itself on perls compiled without taint support.

codecov[bot] commented 2 years ago

Codecov Report

Merging #30 (884ea39) into master (3640eaa) will decrease coverage by 0.02%. The diff coverage is 77.77%.

@@            Coverage Diff             @@
##           master      #30      +/-   ##
==========================================
- Coverage   79.47%   79.45%   -0.03%     
==========================================
  Files          18       19       +1     
  Lines         721      730       +9     
  Branches       79       81       +2     
==========================================
+ Hits          573      580       +7     
  Misses        114      114              
- Partials       34       36       +2     
Impacted Files Coverage Δ
lib/DateTime/Locale/Data.pm 70.83% <0.00%> (-3.08%) :arrow_down:
t/taint.t 87.50% <87.50%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 3640eaa...884ea39. Read the comment docs.

autarch commented 2 years ago

Thanks! I merged this from the CLI with a few tweaks.