← All changes | inc/pdf/receipt-generator.php +105 -5 1.5.1 → 1.6.1 View file →

		@@ -48,9 +48,10 @@
48	48
49	49	if ( ! empty( $existing_relative ) ) {
50	50	$existing_path = self::relative_to_path( Helper::get_string_value( $existing_relative ) );
51	51
52		- if ( $existing_path && file_exists( $existing_path ) ) {
	52	+ // is_file() so a directory can never be served as a cached receipt.
	53	+ if ( $existing_path && is_file( $existing_path ) ) {
53	54	return $existing_path;
54	55	}
55	56	}
56	57
		@@ -271,15 +272,25 @@
271	272	* Used by the personal-data eraser: the receipt is generated from the donor's
272	273	* name/email/address, so an erasure must remove the file from disk, not just
273	274	* the database columns.
274	275	*
	276	+ * A path that fails containment is refused rather than deleted, and still
	277	+ * reports true: the caller's row should not be blocked forever by a
	278	+ * pointer this function will never act on, and refusing to touch the file
	279	+ * is the safe half of the trade.
	280	+ *
275	281	* @since 1.2.0
276	282	* @param string $relative_path Relative path within the uploads directory.
277		- * @return bool True when no file remains (deleted or never existed), false when it survived deletion.
	283	+ * @return bool True when this function will do nothing further with the path (file removed, never existed, or refused as out of bounds), false when the file survived deletion.
278	284	*/
279	285	public static function delete_receipt( $relative_path ) {
280	286	$filepath = self::relative_to_path( $relative_path );
281	287
	288	+ // file_exists() here, is_file() below, and the asymmetry is deliberate:
	289	+ // PHPStan narrows a repeated identical call, so guarding with is_file()
	290	+ // makes the post-delete is_file() read as always-false to it. The two
	291	+ // answers only differ for a directory named *.pdf, and that returns true
	292	+ // either way (nothing that is a file remains), so nothing is lost.
282	293	if ( false === $filepath || ! file_exists( $filepath ) ) {
283	294	return true;
284	295	}
285	296
		@@ -497,10 +508,14 @@
497	508
498	509	/**
499	510	* Convert a relative path to an absolute file path.
500	511	*
	512	+ * Refuses any value that does not resolve to a plain file inside the
	513	+ * receipts directory, so a pointer that ever became attacker-influenced
	514	+ * cannot reach an arbitrary path through either consumer.
	515	+ *
501	516	* @param string $relative_path Relative path within the uploads directory.
502		- * @return string|false Absolute file path or false.
	517	+ * @return string|false Absolute file path inside the receipts directory, or false.
503	518	* @since 1.0.0
504	519	*/
505	520	private static function relative_to_path( $relative_path ) {
506	521	if ( empty( $relative_path ) ) {
		@@ -506,11 +521,96 @@
506	521	if ( empty( $relative_path ) ) {
507	522	return false;
508	523	}
509	524
510		- $upload_dir = wp_upload_dir();
	525	+ // Everything below is containment for a value this function does not
	526	+ // own. The column is written only by generate() today, and nothing
	527	+ // sanitizes it on the way into the database, so the stored string is
	528	+ // trusted purely because no write path currently exposes it. Both
	529	+ // consumers are destructive if that ever stops being true: this feeds
	530	+ // wp_delete_file() on every donation delete, and get_or_generate()
	531	+ // hands the resolved path to the donor-facing receipt download. Check
	532	+ // it here, once, rather than relying on every future caller.
	533	+ $normalized = wp_normalize_path( (string) $relative_path );
511	534
512		- return $upload_dir['basedir'] . '/' . $relative_path;
	535	+ // A null byte truncates the path inside the C filesystem calls.
	536	+ if ( false !== strpos( $normalized, "\0" ) ) {
	537	+ return false;
	538	+ }
	539	+
	540	+ // A literal backslash, refused on the raw value before anything else
	541	+ // reads it. Normalization treats it as a separator, so the checks below
	542	+ // would measure a different path from the one this function returns, and
	543	+ // the realpath comparison would normalize it back again. No value the
	544	+ // generator has ever written contains one: the stored string is built
	545	+ // with '/' and sanitize_file_name() strips '\' from the filename.
	546	+ if ( false !== strpos( (string) $relative_path, '\\' ) ) {
	547	+ return false;
	548	+ }
	549	+
	550	+ // Traversal, in any position. This does not stand alone: a segment such
	551	+ // as '.. ' is not matched here, and Windows strips trailing spaces
	552	+ // during path canonicalisation. The realpath() cross-check below is
	553	+ // what covers those, so do not remove it as redundant.
	554	+ if ( preg_match( '#(^|/)\.\.(/|$)#', $normalized ) ) {
	555	+ return false;
	556	+ }
	557	+
	558	+ // Already absolute: a POSIX root, a Windows drive, or a stream wrapper
	559	+ // such as phar:// or http://. None can be a relative receipt path.
	560	+ if ( 0 === strpos( $normalized, '/' ) || preg_match( '#^[a-zA-Z]:/#', $normalized ) || preg_match( '#^[a-zA-Z][a-zA-Z0-9+.-]*://#', $normalized ) ) {
	561	+ return false;
	562	+ }
	563	+
	564	+ // One wp_upload_dir() call feeds both sides. The upload_dir filter runs
	565	+ // on every call, so fetching twice lets a filter that varies its answer
	566	+ // desynchronise the candidate from the directory it is measured against.
	567	+ $upload_dir = wp_upload_dir();
	568	+ $base_dir = wp_normalize_path( $upload_dir['basedir'] );
	569	+ $receipts_dir = $base_dir . '/suredonation/receipts';
	570	+ $candidate = wp_normalize_path( $base_dir . '/' . $normalized );
	571	+
	572	+ // Receipts live in exactly one directory. With traversal already
	573	+ // refused above, a prefix test is a containment test.
	574	+ if ( 0 !== strpos( $candidate, $receipts_dir . '/' ) ) {
	575	+ return false;
	576	+ }
	577	+
	578	+ // Inside the directory is not enough: it also holds the .htaccess,
	579	+ // index.php and web.config that ensure_receipts_dir() writes to keep it
	580	+ // from being served. Resolving one of those would let a delete strip
	581	+ // the directory's protection and expose every donor receipt, which is a
	582	+ // worse outcome than the arbitrary delete this containment exists to
	583	+ // stop. Every name the generator can produce ends in .pdf.
	584	+ $basename = basename( $normalized );
	585	+ if ( '' === $basename || 0 === strpos( $basename, '.' ) || ! preg_match( '#\.pdf\z#i', $basename ) ) {
	586	+ return false;
	587	+ }
	588	+
	589	+ // Built from the raw value, because this is what the function returns
	590	+ // and therefore what the callers act on. Keeping the returned string
	591	+ // byte-identical to the old behaviour matters (get_or_generate() hands
	592	+ // it back and it is compared), but the realpath check below has to
	593	+ // measure that same string: normalization turns a literal backslash
	594	+ // into a separator, so checking only the normalized form would leave a
	595	+ // symlink named with one unexamined.
	596	+ $filepath = $upload_dir['basedir'] . '/' . $relative_path;
	597	+
	598	+ // A symlink can still point out of the directory, and realpath() is the
	599	+ // only thing that sees it. It resolves to false when the file is not
	600	+ // there yet, which is a normal state for both callers, so only an
	601	+ // existing file is cross-checked. Both sides are resolved so that a
	602	+ // symlinked uploads directory, which is common when media sits on
	603	+ // another volume, does not cause a false refusal.
	604	+ $real_path = realpath( $filepath );
	605	+ if ( false !== $real_path ) {
	606	+ $real_dir = realpath( $receipts_dir );
	607	+ if ( false === $real_dir || 0 !== strpos( wp_normalize_path( $real_path ), wp_normalize_path( $real_dir ) . '/' ) ) {
	608	+ return false;
	609	+ }
	610	+ }
	611	+
	612	+ return $filepath;
513	613	}
514	614
515	615	/**
516	616	* Resolve the filename the receipt is stored under on disk.