Merge pull request #7601 from LibreSign/backport/7598/stable33

[stable33] fix(validation): harden signed file validation handling

Author: vitormattos

lib/Service/File/CertificateChainService.php

@@ -27,6 +27,10 @@ public function getCertificateChain($fileNode, File $libreSignFile, $options): a
 
 		try {
 			$resource = $fileNode->fopen('rb');
+			if (!is_resource($resource)) {
+				$this->logger->warning('Failed to load certificate chain: unable to open signed file stream');
+				return [];
+			}
 			$sha256 = $this->getSha256FromResource($resource);
 			rewind($resource);
 			if ($sha256 === $libreSignFile->getSignedHash()) {
@@ -42,9 +46,16 @@ public function getCertificateChain($fileNode, File $libreSignFile, $options): a
 	}
 
 	private function getSha256FromResource($resource): string {
+		if (!is_resource($resource)) {
+			return '';
+		}
+
 		$hashContext = hash_init('sha256');
 		while (!feof($resource)) {
 			$buffer = fread($resource, 8192);
+			if ($buffer === false) {
+				break;
+			}
 			hash_update($hashContext, $buffer);
 		}
 		return hash_final($hashContext);

lib/Service/File/EnvelopeAssembler.php

@@ -143,6 +143,9 @@ public function buildEnvelopeChildData(File $childFile, \OCA\Libresign\Service\F
 						$certData = $this->certificateChainService->getCertificateChain($fileNode, $childFile, $options);
 					} else {
 						$resource = $fileNode->fopen('rb');
+						if (!is_resource($resource)) {
+							throw new \RuntimeException('unable to open signed file stream');
+						}
 						$sha256 = $this->getSha256FromResource($resource);
 						rewind($resource);
 						if ($sha256 === $childFile->getSignedHash()) {
@@ -164,9 +167,16 @@ public function buildEnvelopeChildData(File $childFile, \OCA\Libresign\Service\F
 	}
 
 	private function getSha256FromResource($resource): string {
+		if (!is_resource($resource)) {
+			return '';
+		}
+
 		$hashContext = hash_init('sha256');
 		while (!feof($resource)) {
 			$buffer = fread($resource, 8192);
+			if ($buffer === false) {
+				break;
+			}
 			hash_update($hashContext, $buffer);
 		}
 		return hash_final($hashContext);

src/services/validationDocument.ts

@@ -64,7 +64,15 @@ function isOptionalField(record: UnknownRecord, key: string, guard: (value: unkn
 }
 
 function toNumber(value: unknown): number | null {
-	return typeof value === 'number' && Number.isFinite(value) ? value : null
+	if (typeof value === 'number' && Number.isFinite(value)) {
+		return value
+	}
+
+	if (typeof value === 'string' && /^-?\d+$/.test(value)) {
+		return Number.parseInt(value, 10)
+	}
+
+	return null
 }
 
 function isString(value: unknown): value is string {
@@ -91,6 +99,15 @@ function isSignerStatus(value: unknown): value is SignerDetailRecord['status'] {
 		|| normalizedValue === SIGN_REQUEST_STATUS.SIGNED
 }
 
+function isValidationSignatureFlow(value: unknown): boolean {
+	if (value === 'none' || value === 'parallel' || value === 'ordered_numeric') {
+		return true
+	}
+
+	const normalizedValue = toNumber(value)
+	return normalizedValue === 0 || normalizedValue === 1 || normalizedValue === 2
+}
+
 function isValidationStatusInfo(value: unknown): value is ValidationStatusInfo {
 	if (!isRecord(value)) {
 		return false
@@ -210,12 +227,12 @@ function isValidationDocumentRecord(data: unknown): data is ValidationFileRecord
 		|| !isString(data.statusText)
 		|| typeof data.nodeId !== 'number'
 		|| (data.nodeType !== 'file' && data.nodeType !== 'envelope')
-		|| typeof data.signatureFlow !== 'number'
-		|| typeof data.docmdpLevel !== 'number'
-		|| typeof data.filesCount !== 'number'
+		|| !isValidationSignatureFlow(data.signatureFlow)
+		|| toNumber(data.docmdpLevel) === null
+		|| toNumber(data.filesCount) === null
 		|| !Array.isArray(data.files)
-		|| typeof data.totalPages !== 'number'
-		|| typeof data.size !== 'number'
+		|| toNumber(data.totalPages) === null
+		|| toNumber(data.size) === null
 		|| !isString(data.pdfVersion)
 		|| !isString(data.created_at)
 		|| !isRequestedBy(data.requested_by)

src/tests/services/validationDocument.spec.ts

@@ -106,6 +106,20 @@ describe('validationDocument', () => {
 		}))
 	})
 
+	it('accepts validation payload when signatureFlow is enum string and numeric fields are numeric strings', () => {
+		const normalized = toValidationDocument(createValidationPayload({
+			signatureFlow: 'none',
+			docmdpLevel: '2',
+			filesCount: '1',
+			totalPages: '1',
+			size: '10',
+			signers: [createSigner({ status: '2' })],
+		}))
+
+		expect(normalized).not.toBeNull()
+		expect(normalized?.signatureFlow).toBe('none')
+	})
+
 	it('rejects payload with invalid signer status', () => {
 		const normalized = toValidationDocument(createValidationPayload({
 			signers: [createSigner({ status: 99, statusText: 'Invalid' })],

tests/php/Unit/Service/File/CertificateChainServiceTest.php

@@ -53,4 +53,33 @@ public function fopen($mode) {
 		$this->assertIsArray($result);
 		$this->assertArrayHasKey('chain', $result);
 	}
+
+	public function testGetCertificateChainHandlesInvalidResourceGracefully(): void {
+		$fileNode = new class() {
+			public function fopen($mode) {
+				return false;
+			}
+		};
+
+		$libreSignFile = new File();
+		$libreSignFile->setSignedNodeId(1);
+
+		$pkcs12 = $this->createMock(Pkcs12Handler::class);
+		$pkcs12->expects($this->never())->method('getCertificateChain');
+
+		$logger = $this->createMock(LoggerInterface::class);
+		$logger
+			->expects($this->once())
+			->method('warning')
+			->with($this->stringContains('unable to open signed file stream'));
+
+		$service = new CertificateChainService($pkcs12, $logger);
+
+		$options = new FileResponseOptions();
+		$options->validateFile(true);
+
+		$result = $service->getCertificateChain($fileNode, $libreSignFile, $options);
+
+		$this->assertSame([], $result);
+	}
 }