5250ng / 5250ng/5250script

readFieldText does not validate row/col against screen bounds

Open
#4 0 comments 0 reactions 1 assignee View on GitHub

@p0dalirius is already working on this.

Since Apr 18, 2026.

bug
Dominant language
C++
Stars
0
Forks
1
PR merge metrics
No merged PRs in 30d

Description

Summary

ScriptExecutor::readFieldText(row, col) forwards the caller-supplied row and col to the ScreenInterface without validating them against the screen's rows() / cols(). This is inconsistent with the sibling helper readScreenText(row, col, length) which does range-check row and rejects out-of-range coordinates before dispatching. As a result, EXPECT FIELD AT ... and EXTRACT $var FIELD AT ... pass negative or out-of-screen coordinates straight to the embedding application's screen implementation.

Location
  • File: src/script_executor.cpp
  • Lines / functions:
    • readFieldText() at L805–L808 (no bounds check)
    • Compare readScreenText() at L799–L802 (does bounds-check row and reject negative col)
    • Callers: ExtractType::FieldAt at L513, ExpectType::FieldContains at L591
Category

error-handling

Severity

medium

Impact: malformed coordinates (e.g., EXPECT FIELD AT 999 999 CONTAINS "X") cross the library boundary into the host's ScreenInterface implementation. Whether this crashes or silently misbehaves depends on the host — but the library should not rely on host-side validation when it already validates similar inputs for a neighbouring helper.

Reproduction / Evidence

Verified by code analysis.

readScreenText(row, col, length) (L799):

if (row < 0 || row >= m_screen->rows() || col < 0) return {};
return m_screen->readText(row, col, length);

readFieldText(row, col) (L805):

if (!m_screen) return {};
return m_screen->readFieldText(row, col);   // no row/col validation

A script like EXPECT FIELD AT -1 -1 CONTAINS "x" converts to readFieldText(-2, -2) (1-based → 0-based subtraction at L513/L591) and hands (-2, -2) directly to the host screen.

Expected Behavior

readFieldText should perform the same bounds check as readScreenText and return an empty QString for out-of-range coordinates. The EXPECT FIELD comparison and EXTRACT FIELD extraction should then treat the field as empty / not-found, not trigger unpredictable host behavior.

Actual Behavior

Out-of-range coordinates are forwarded to the host's ScreenInterface::readFieldText, producing host-dependent behavior (potential crash, potential garbage).

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.