Skip to content

Comments

Harden ProgressBar against local DoS from unbounded width/maxval and widget output#86

Merged
NiltonVolpato merged 3 commits intomasterfrom
copilot/fix-dos-vulnerability-progressbar
Feb 24, 2026
Merged

Harden ProgressBar against local DoS from unbounded width/maxval and widget output#86
NiltonVolpato merged 3 commits intomasterfrom
copilot/fix-dos-vulnerability-progressbar

Conversation

Copy link
Contributor

Copilot AI commented Feb 24, 2026 •
edited
Loading

progressbar accepted unbounded term_width, maxval, and widget output, allowing local resource exhaustion (large allocations and expensive arithmetic) through constructor args, environment (COLUMNS), and resize handling. This change introduces strict bounds and normalization on those high-amplification paths while preserving existing API behavior.

  • Input bounds in ProgressBar

    • Added ProgressBar._MAX_TERMSIZE = 4096 and ProgressBar._MAX_MAXVAL = 10**12.
    • Added _sanitize_term_width() and _sanitize_maxval() and applied them to:
      • explicit constructor args (term_width, maxval)
      • iterable-length inference in __call__
      • environment-derived width in _env_size()
      • signal/TTY-derived width in _handle_resize()
    • Oversized/invalid maxval now raises ValueError with specific messages; terminal width is clamped.

  • Widget output size guard

    • Added widgets._MAX_UPDATABLE_LENGTH = 4096.
    • format_updatable() now normalizes output (None/non-string) and truncates oversized strings before formatting/joining.

  • Focused regression coverage

    • Added tests for:
      • explicit oversized term_width clamping
      • oversized COLUMNS clamping
      • rejection of extreme maxval
      • truncation of oversized widget output
class ProgressBar(object):
    _MAX_MAXVAL = 1000000000000
    _MAX_TERMSIZE = 4096

    @classmethod
    def _sanitize_term_width(cls, term_width):
        if term_width < 1:
            return 1
        return min(term_width, cls._MAX_TERMSIZE)

def format_updatable(updatable, pbar):
    output = updatable.update(pbar) if hasattr(updatable, 'update') else updatable
    output = '' if output is None else str(output)
    return output[:_MAX_UPDATABLE_LENGTH]
Original prompt

This section details on the original issue you should resolve

<issue_title>Multiple Local Denial of Service (DoS) vulnerabilities via resource exhaustion</issue_title>
<issue_description>Description:
The progressbar library contains multiple input validation flaws that allow local Denial of Service (DoS) attacks. An attacker can cause memory exhaustion or CPU overload by passing extreme values to constructor parameters.

Tested Environment:

  • OS: Arch Linux (virtual machine)

  • Memory: 3.7GB RAM

Vulnerability Details:

FLAW #1: term_width Parameter Without Validation

def __init__(self, maxval=None, widgets=None, term_width=None, poll=1,
             left_justify=True, fd=None):
    
    if term_width is not None:
        self.term_width = term_width  #  NO VALIDATION! Accepts any value
    else:
        try:
            self._handle_resize()
            signal.signal(signal.SIGWINCH, self._handle_resize)
            self.signal_set = True
        except (SystemExit, KeyboardInterrupt): raise
        except:
            self.term_width = self._env_size()  #  Can be manipulated via environment variable

....

def _format_line(self):

    """Joins the widgets and justifies the line."""
    widgets = ''.join(self._format_widgets())
    
    if self.left_justify: 
        return widgets.ljust(self.term_width)  #  CREATES HUGE STRING!
    else: 
        return widgets.rjust(self.term_width)   # SAME PROBLEM

Why it happens:

term_width is used directly in ljust()/rjust() without size validation

If term_width = 1,000,000, each update creates a 1MB string

After 100 updates → 100MB allocated → MemoryError and crash

FLAW #2: maxval Parameter Without Validation

def percentage(self):
    """Calculates the percentage of progress."""
    if self.maxval is widgets.UnknownLength:
        return float("NaN")
    if self.currval >= self.maxval:
        return 100.0
    return (self.currval * 100.0 / self.maxval) if self.maxval else 100.00  #  DIVISION WITH HUGE NUMBERS

Why it happens:

maxval can be 10**100 (googol) without validation

Each update calculates currval * 100.0 / maxval

Divisions with astronomical numbers consume excessive CPU

Result: 100% CPU usage, slow system, timeouts

FLAW #3: Iterator Without Iteration Limit

def __next__(self):
    try:
        value = next(self.__iterable)  # ❌ CAN BE INFINITE
        if self.start_time is None:
            self.start()
        else:
            self.update(self.currval + 1)  # ❌ CALLED FOREVER
        return value
    except StopIteration:
        if self.start_time is None:
            self.start()
        self.finish()
        raise

Why it happens:

__iterable can be an infinite generator (while True: yield)

update() is called on each iteration → cumulative resource consumption

Result: Infinite loop + continuous allocation → memory/CPU crash

FLAW #4: Interval Calculation Without Protection

def start(self):
    self.num_intervals = max(100, self.term_width)  #  CAN BE HUGE
    
    if self.maxval is not widgets.UnknownLength:
        if self.maxval < 0: raise ValueError('Value out of range')
        self.update_interval = self.maxval / self.num_intervals  #  DIVISION WITH EXTREME VALUES

Why it happens:

num_intervals = max(100, term_width) → if term_width=10^6, num_intervals=10^6

update_interval = maxval / num_intervals → if maxval=10^100, division with huge numbers

Result: Heavy calculations on each update

FLAW #5: Widgets Can Return Huge Strings

def format_updatable(updatable, pbar):
    if hasattr(updatable, 'update'): 
        return updatable.update(pbar)  # ❌ CAN RETURN 1GB STRING
    else: 
        return updatable

Why it happens:

Widgets can implement update() without limits

Return value is concatenated into the final string

Result: Exponential memory allocation

FLAW #6: Environment Can Be Manipulated

def _env_size(self):
    """Tries to find the term_width from the environment."""
    return int(os.environ.get('COLUMNS', self._DEFAULT_TERMSIZE)) - 1  #  UNTRUSTED ENVIRONMENT VARIABLE

Why it happens:

Attacker can set COLUMNS=1000000 in the environment

term_width will be set to 999999

Result: DoS via environment without passing direct parameters

FLAW #7: Signal Handler Without Protection

def _handle_resize(self, signum=None, frame=None):
    """Tries to catch resize signals sent from the terminal."""
    h, w = array('h', ioctl(self.fd, termios.TIOCGWINSZ, '\0' * 8))[:2]  #  FIXED BUFFER, POSSIBLE OVERFLOW
    self.term_width = w  #  CAN BE HUGE

Why it happens:

ioctl() with '\0'*8 buffer may not be safe

If w is huge (e.g., 65535), term_width becomes gigantic

Result: Next update creates massive string

FLA...

🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

Copilot AI and others added 2 commits February 24, 2026 07:20
@NiltonVolpato
Harden ProgressBar against DoS-sized inputs
2d0c37f 
Co-authored-by: NiltonVolpato <761595+NiltonVolpato@users.noreply.github.com>
@NiltonVolpato
Address review feedback for DoS hardening
80cc3a6 
Co-authored-by: NiltonVolpato <761595+NiltonVolpato@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix DoS vulnerabilities via resource exhaustion in progressbar Harden ProgressBar against local DoS from unbounded width/maxval and widget output Feb 24, 2026
Copilot AI requested a review from NiltonVolpato February 24, 2026 07:23
@NiltonVolpato NiltonVolpato marked this pull request as ready for review February 24, 2026 08:22
@NiltonVolpato NiltonVolpato merged commit 279911b into master Feb 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@NiltonVolpato NiltonVolpato NiltonVolpato approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@NiltonVolpato NiltonVolpato

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Multiple Local Denial of Service (DoS) vulnerabilities via resource exhaustion

2 participants

@NiltonVolpato