diff --git a/.changeset/ocr-service-drop-node-tesseract-ocr.md b/.changeset/ocr-service-drop-node-tesseract-ocr.md new file mode 100644 index 00000000..fca82b25 --- /dev/null +++ b/.changeset/ocr-service-drop-node-tesseract-ocr.md @@ -0,0 +1,7 @@ +--- +'@wdio/ocr-service': patch +--- + +Remove the `node-tesseract-ocr` dependency from `@wdio/ocr-service`. It carried an unpatched critical advisory (GHSA-8j44-735h-w4w2, OS command injection in `recognize()`) and has had no release since 2022, so every project that installed the OCR service got one `npm audit` finding it could not clear. + +The service now runs the system `tesseract` binary itself through `child_process.execFile` with an argument array, so the screenshot path and the options never pass through a shell. The command that is run is the same as before (`tesseract stdout -l --oem 1 --psm 3 alto`), so the OCR output does not change. `isSystemTesseractAvailable()` uses `execFileSync` for the same reason. diff --git a/packages/ocr-service/package.json b/packages/ocr-service/package.json index 353bb549..a2c8cab8 100644 --- a/packages/ocr-service/package.json +++ b/packages/ocr-service/package.json @@ -37,7 +37,6 @@ "@wdio/types": "^10.0.1", "fuse.js": "^7.5.0", "jimp": "^1.6.1", - "node-tesseract-ocr": "^2.2.1", "tesseract.js": "^7.0.0", "xml2js": "^0.6.2" }, diff --git a/packages/ocr-service/src/utils/tesseract.ts b/packages/ocr-service/src/utils/tesseract.ts index 1c4b9b9a..fe8436c3 100644 --- a/packages/ocr-service/src/utils/tesseract.ts +++ b/packages/ocr-service/src/utils/tesseract.ts @@ -1,15 +1,21 @@ -import { execSync } from 'node:child_process' +import { execFile, execFileSync } from 'node:child_process' +import { promisify } from 'node:util' import { createWorker, PSM } from 'tesseract.js' -import { recognize } from 'node-tesseract-ocr' import { parseString } from 'xml2js' import type { GetOcrData, Line, ParseWordData, Rectangles, TessaractDataOptions, UnprocessedNodejsBlock, UnprocessedSystemBlock, Words } from '../types.js' +const execFileAsync = promisify(execFile) +// Tesseract writes the ALTO XML of a full-screen capture to stdout. 1 MB (the Node +// default) is too small for a busy screen, so give it room. +const TESSERACT_MAX_STDOUT_BYTES = 10 * 1024 * 1024 + export function isSystemTesseractAvailable(tesseractName: string = ''): boolean { const binary = tesseractName || 'tesseract' - const command = [binary, '--version'].join(' ') try { - execSync(command) + // execFileSync with an argument array never starts a shell, so the binary + // name is passed through as-is. + execFileSync(binary, ['--version'], { stdio: 'ignore' }) } catch (_ign) { return false } @@ -17,6 +23,30 @@ export function isSystemTesseractAvailable(tesseractName: string = ''): boolean return true } +type SystemTesseractOptions = { + lang: string; + oem: number; + psm: number; + presets: string[]; +} + +/** + * Run the system Tesseract binary and return what it writes to stdout. + * + * The file path and every option go to the binary as an argument array, so no + * shell ever sees them. This replaces `node-tesseract-ocr`, which built one shell + * command string from the file path (GHSA-8j44-735h-w4w2) and has no patched + * release. + */ +export async function runSystemTesseract(filePath: string, { lang, oem, psm, presets }: SystemTesseractOptions): Promise { + // `-l` and `--psm` must come before any config file name such as `alto`, see + // https://github.com/tesseract-ocr/tesseract/blob/master/doc/tesseract.1.asc + const args = [filePath, 'stdout', '-l', lang, '--oem', String(oem), '--psm', String(psm), ...presets] + const { stdout } = await execFileAsync('tesseract', args, { maxBuffer: TESSERACT_MAX_STDOUT_BYTES }) + + return stdout +} + export function parseWordDataFromText(attributes: string[]): ParseWordData { let bbox = { left: 0, @@ -164,10 +194,9 @@ export async function getSystemOcrData(options: TessaractDataOptions): Promise ({ - execSync: vi.fn() + execFile: vi.fn(), + execFileSync: vi.fn() })) vi.mock('tesseract.js', () => ({ createWorker: vi.fn(() => ({ @@ -21,9 +21,6 @@ vi.mock('tesseract.js', () => ({ AUTO: 'auto' } })) -vi.mock('node-tesseract-ocr', () => ({ - recognize: vi.fn() -})) vi.mock('xml2js', () => ({ parseString: vi.fn() })) @@ -34,32 +31,87 @@ describe('isSystemTesseractAvailable', () => { }) it('returns true when Tesseract is available', () => { - vi.mocked(execSync).mockImplementation(() => '') + vi.mocked(execFileSync).mockImplementation(() => '') expect(isSystemTesseractAvailable()).toBe(true) - expect(execSync).toHaveBeenCalledWith('tesseract --version') + expect(execFileSync).toHaveBeenCalledWith('tesseract', ['--version'], { stdio: 'ignore' }) }) it('returns false when Tesseract is not available', () => { - vi.mocked(execSync).mockImplementation(() => { + vi.mocked(execFileSync).mockImplementation(() => { throw new Error('command not found') }) expect(isSystemTesseractAvailable()).toBe(false) - expect(execSync).toHaveBeenCalledWith('tesseract --version') + expect(execFileSync).toHaveBeenCalledWith('tesseract', ['--version'], { stdio: 'ignore' }) }) it('checks the specified Tesseract binary name', () => { const customBinary = 'custom-tesseract' // @ts-ignore - vi.mocked(execSync).mockImplementation((command) => { - if (!command.includes(customBinary)) { + vi.mocked(execFileSync).mockImplementation((file) => { + if (file !== customBinary) { throw new Error('command not found') } }) expect(isSystemTesseractAvailable(customBinary)).toBe(true) - expect(execSync).toHaveBeenCalledWith(`${customBinary} --version`) + expect(execFileSync).toHaveBeenCalledWith(customBinary, ['--version'], { stdio: 'ignore' }) + }) + + it('never starts a shell, so a binary name with shell characters is passed through literally', () => { + const hostileBinary = 'tesseract; touch /tmp/pwned' + vi.mocked(execFileSync).mockImplementation(() => '') + + isSystemTesseractAvailable(hostileBinary) + + expect(execFileSync).toHaveBeenCalledWith(hostileBinary, ['--version'], { stdio: 'ignore' }) + }) +}) + +describe('runSystemTesseract', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('runs the tesseract binary with an argument array and returns stdout', async () => { + // @ts-ignore + vi.mocked(execFile).mockImplementation((_file, _args, _options, callback) => { + callback(null, { stdout: '', stderr: '' }) + }) + + const result = await runSystemTesseract('/fake/path/image.png', { lang: 'eng', oem: 1, psm: 3, presets: ['alto'] }) + + expect(result).toBe('') + expect(execFile).toHaveBeenCalledWith( + 'tesseract', + ['/fake/path/image.png', 'stdout', '-l', 'eng', '--oem', '1', '--psm', '3', 'alto'], + { maxBuffer: 10 * 1024 * 1024 }, + expect.any(Function), + ) + }) + + it('passes a file path with shell characters through as one literal argument', async () => { + const hostilePath = '/tmp/image.png"; touch /tmp/pwned; echo "' + // @ts-ignore + vi.mocked(execFile).mockImplementation((_file, _args, _options, callback) => { + callback(null, { stdout: '', stderr: '' }) + }) + + await runSystemTesseract(hostilePath, { lang: 'eng', oem: 1, psm: 3, presets: ['alto'] }) + + const [, args] = vi.mocked(execFile).mock.calls[0] + expect(args[0]).toBe(hostilePath) + }) + + it('rejects when the binary fails', async () => { + // @ts-ignore + vi.mocked(execFile).mockImplementation((_file, _args, _options, callback) => { + callback(new Error('Recognition failed'), { stdout: '', stderr: '' }) + }) + + await expect(runSystemTesseract('/fake/path/image.png', { lang: 'eng', oem: 1, psm: 3, presets: ['alto'] })) + .rejects.toThrow('Recognition failed') }) }) @@ -334,6 +386,13 @@ describe('getSystemOcrData', () => { const mockLanguage = 'eng' const options = { filePath: mockFilePath, language: mockLanguage } + function mockTesseractStdout(stdout: string) { + // @ts-ignore + vi.mocked(execFile).mockImplementation((_file, _args, _options, callback) => { + callback(null, { stdout, stderr: '' }) + }) + } + beforeEach(() => { vi.clearAllMocks() mockedParsedTextSting = '\n\t\t\nNext-gen browser and mobile automation\n\ntest framework for Node.js\n\t' @@ -376,7 +435,7 @@ describe('getSystemOcrData', () => { it('successfully processes OCR data', async () => { const mockResult = '' - vi.mocked(recognize).mockResolvedValue(mockResult) + mockTesseractStdout(mockResult) vi.mocked(parseString).mockImplementation((_input, callback) => { // @ts-ignore callback(null, { @@ -391,19 +450,22 @@ describe('getSystemOcrData', () => { const result = await getSystemOcrData(options) - expect(recognize).toHaveBeenCalledWith(mockFilePath, { - lang: mockLanguage, - oem: 1, - psm: 3, - presets: ['alto'] - }) + expect(execFile).toHaveBeenCalledWith( + 'tesseract', + [mockFilePath, 'stdout', '-l', mockLanguage, '--oem', '1', '--psm', '3', 'alto'], + expect.any(Object), + expect.any(Function), + ) expect(result.text).toMatchSnapshot() expect(result.lines.length).toEqual(2) expect(result.words.length).toEqual(4) }) it('handles recognition errors', async () => { - vi.mocked(recognize).mockRejectedValue(new Error('Recognition failed')) + // @ts-ignore + vi.mocked(execFile).mockImplementation((_file, _args, _options, callback) => { + callback(new Error('Recognition failed'), { stdout: '', stderr: '' }) + }) await expect(getSystemOcrData({ filePath: mockFilePath, language: mockLanguage })) .rejects @@ -413,7 +475,7 @@ describe('getSystemOcrData', () => { it('handles XML parsing errors', async () => { const mockResult = 'Invalid XML content' - vi.mocked(recognize).mockResolvedValue(mockResult) + mockTesseractStdout(mockResult) vi.mocked(parseString).mockImplementation((_input, callback) => { // @ts-ignore callback(new Error('Parsing failed'), null) @@ -427,7 +489,7 @@ describe('getSystemOcrData', () => { it('handles cases with no text found', async () => { const mockResult = '' - vi.mocked(recognize).mockResolvedValue(mockResult) + mockTesseractStdout(mockResult) vi.mocked(parseString).mockImplementation((_input, callback) => { // @ts-ignore callback(null, { alto: { Layout: [{ Page: [{ PrintSpace: [{ ComposedBlock: [] }] }] }] } }) @@ -444,7 +506,7 @@ describe('getSystemOcrData', () => { beforeEach(() => { vi.clearAllMocks() - vi.mocked(recognize).mockResolvedValue('...') // Base mock for OCR recognize function + mockTesseractStdout('...') // Base mock for the tesseract binary }) function setupStringMock(missingKey) { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 27e0192b..c253880d 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -165,9 +165,6 @@ importers: jimp: specifier: ^1.6.1 version: 1.6.1(supports-color@8.1.1) - node-tesseract-ocr: - specifier: ^2.2.1 - version: 2.2.1 tesseract.js: specifier: ^7.0.0 version: 7.0.0 @@ -3879,10 +3876,6 @@ packages: node-request-interceptor@0.6.3: resolution: {integrity: sha512-8I2V7H2Ch0NvW7qWcjmS0/9Lhr0T6x7RD6PDirhvWEkUQvy83x8BA4haYMr09r/rig7hcgYSjYh6cd4U7G1vLA==} - node-tesseract-ocr@2.2.1: - resolution: {integrity: sha512-Q9cD79JGpPNQBxbi1fV+OAsTxYKLpx22sagsxSyKbu1u+t6UarApf5m32uVc8a5QAP1Wk7fIPN0aJFGGEE9DyQ==} - engines: {node: '>=10'} - non-error@0.1.0: resolution: {integrity: sha512-TMB1uHiGsHRGv1uYclfhivcnf0/PdFp2pNqRxXjncaAsjYMoisaQJI+SSZCqRq+VliwRTC8tsMQfmrWjDMhkPQ==} engines: {node: '>=20'} @@ -9180,8 +9173,6 @@ snapshots: transitivePeerDependencies: - supports-color - node-tesseract-ocr@2.2.1: {} - non-error@0.1.0: {} normalize-package-data@8.0.0: