Skip to content

Commit 9c63a99

Browse files
macropay-solutionsmacropay-solutions
authored
Merge pull request #9 from macropay-solutions/fix-eagerloading-data-leak-issue-51825
Fix laravel/framework#51825
2 parents 9d4de88 + d4f590d commit 9c63a99

File tree

7 files changed

+107
-39
lines changed

7 files changed

+107
-39
lines changed

‎illuminate/Database/Eloquent/Builder.php

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -808,11 +808,14 @@ public function getRelation($name)
808808
// and error prone. We don't want constraints because we add eager ones.
809809
$relation = Relation::noConstraints(function () use ($name) {
810810
try {
811-
return $this->getModel()->newInstance()->$name();
811+
$model = $this->getModel()->newInstance();
812+
$model->nowEagerLoadingRelationNameWithNoConstraints = $name;
813+
814+
return $model->$name();
812815
} catch (BadMethodCallException) {
813816
throw RelationNotFoundException::make($this->getModel(), $name);
814817
}
815-
});
818+
}, $name);
816819

817820
$nested = $this->relationsNestedUnder($name);
818821

‎illuminate/Database/Eloquent/Concerns/HasRelationships.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@
2525

2626
trait HasRelationships
2727
{
28+
public ?string $nowEagerLoadingRelationNameWithNoConstraints = null;
29+
2830
/**
2931
* The loaded relationships for the model.
3032
*

‎illuminate/Database/Eloquent/Concerns/QueriesRelationships.php

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -277,12 +277,16 @@ public function hasMorph(
277277
protected function getBelongsToRelation(MorphTo $relation, $type)
278278
{
279279
$belongsTo = Relation::noConstraints(function () use ($relation, $type) {
280-
return $this->model->belongsTo(
280+
$model = $this->getModel();
281+
$model->nowEagerLoadingRelationNameWithNoConstraints = $relation->getRelationName();
282+
283+
return $model->belongsTo(
281284
$type,
282285
$relation->getForeignKeyName(),
283-
$relation->getOwnerKeyName()
286+
$relation->getOwnerKeyName(),
287+
$relation->getRelationName(),
284288
);
285-
});
289+
}, $relation->getRelationName());
286290

287291
$belongsTo->getQuery()->mergeConstraintsFrom($relation->getQuery());
288292

@@ -895,8 +899,11 @@ protected function addWhereCountQuery(QueryBuilder $query, $operator = '>=', $co
895899
protected function getRelationWithoutConstraints($relation)
896900
{
897901
return Relation::noConstraints(function () use ($relation) {
898-
return $this->getModel()->{$relation}();
899-
});
902+
$model = $this->getModel();
903+
$model->nowEagerLoadingRelationNameWithNoConstraints = $relation;
904+
905+
return $model->{$relation}();
906+
}, $relation);
900907
}
901908

902909
/**

‎illuminate/Database/Eloquent/Relations/HasMany.php

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
namespace Illuminate\Database\Eloquent\Relations;
44

55
use Illuminate\Database\Eloquent\Collection;
6+
use Illuminate\Support\Str;
67

78
class HasMany extends HasOneOrMany
89
{
@@ -13,13 +14,21 @@ class HasMany extends HasOneOrMany
1314
*/
1415
public function one()
1516
{
17+
$relationName = Str::uuid()->toString();
18+
1619
// return HasOne::noConstraints(fn () => new HasOne(
17-
return HasOne::noConstraints(fn() => \app(HasOne::class, [
18-
$this->getQuery(),
19-
$this->parent,
20-
$this->foreignKey,
21-
$this->localKey,
22-
]));
20+
return HasOne::noConstraints(function () use ($relationName): HasOne {
21+
$this->parent->nowEagerLoadingRelationNameWithNoConstraints = $relationName;
22+
$builder = $this->getQuery()->clone();
23+
$builder->setQuery($builder->getQuery()->clone());
24+
25+
return \app(HasOne::class, [
26+
$builder,
27+
$this->parent,
28+
$this->foreignKey,
29+
$this->localKey,
30+
]);
31+
}, $relationName);
2332
}
2433

2534
/**

‎illuminate/Database/Eloquent/Relations/HasManyThrough.php

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
use Illuminate\Database\Eloquent\Relations\Concerns\InteractsWithDictionary;
1212
use Illuminate\Database\Eloquent\SoftDeletes;
1313
use Illuminate\Database\UniqueConstraintViolationException;
14+
use Illuminate\Support\Str;
1415

1516
class HasManyThrough extends Relation
1617
{
@@ -86,7 +87,7 @@ public function __construct(
8687
$this->throughParent = $throughParent;
8788
$this->secondLocalKey = $secondLocalKey;
8889

89-
parent::__construct($query, $throughParent);
90+
parent::__construct($query, $throughParent, $farParent);
9091
}
9192

9293
/**
@@ -96,16 +97,24 @@ public function __construct(
9697
*/
9798
public function one()
9899
{
100+
$relationName = Str::uuid()->toString();
101+
99102
// return HasOneThrough::noConstraints(fn () => new HasOneThrough(
100-
return HasOneThrough::noConstraints(fn() => \app(HasOneThrough::class, [
101-
$this->getQuery(),
102-
$this->farParent,
103-
$this->throughParent,
104-
$this->getFirstKeyName(),
105-
$this->secondKey,
106-
$this->getLocalKeyName(),
107-
$this->getSecondLocalKeyName(),
108-
]));
103+
return HasOneThrough::noConstraints(function () use ($relationName): HasOneThrough {
104+
$this->farParent->nowEagerLoadingRelationNameWithNoConstraints = $relationName;
105+
$builder = $this->getQuery()->clone();
106+
$builder->setQuery($builder->getQuery()->clone());
107+
108+
return \app(HasOneThrough::class, [
109+
\tap($builder, fn (Builder $query): array => $query->getQuery()->joins = []),
110+
$this->farParent,
111+
$this->throughParent,
112+
$this->getFirstKeyName(),
113+
$this->secondKey,
114+
$this->getLocalKeyName(),
115+
$this->getSecondLocalKeyName(),
116+
]);
117+
}, $relationName);
109118
}
110119

111120
/**

‎illuminate/Database/Eloquent/Relations/MorphMany.php

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
use Illuminate\Database\Eloquent\Collection;
66
use Illuminate\Database\Eloquent\Model;
7+
use Illuminate\Support\Str;
78

89
class MorphMany extends MorphOneOrMany
910
{
@@ -14,14 +15,22 @@ class MorphMany extends MorphOneOrMany
1415
*/
1516
public function one()
1617
{
18+
$relationName = Str::uuid()->toString();
19+
1720
// return MorphOne::noConstraints(fn () => new MorphOne(
18-
return MorphOne::noConstraints(fn() => \app(MorphOne::class, [
19-
$this->getQuery(),
20-
$this->getParent(),
21-
$this->morphType,
22-
$this->foreignKey,
23-
$this->localKey,
24-
]));
21+
return MorphOne::noConstraints(function () use ($relationName): MorphOne {
22+
$this->getParent()->nowEagerLoadingRelationNameWithNoConstraints = $relationName;
23+
$builder = $this->getQuery()->clone();
24+
$builder->setQuery($builder->getQuery()->clone());
25+
26+
return \app(MorphOne::class, [
27+
$builder,
28+
$this->getParent(),
29+
$this->morphType,
30+
$this->foreignKey,
31+
$this->localKey,
32+
]);
33+
}, $relationName);
2534
}
2635

2736
/**

‎illuminate/Database/Eloquent/Relations/Relation.php

Lines changed: 38 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
namespace Illuminate\Database\Eloquent\Relations;
44

5-
use Closure;
65
use Illuminate\Contracts\Database\Eloquent\Builder as BuilderContract;
76
use Illuminate\Database\Eloquent\Builder;
87
use Illuminate\Database\Eloquent\Collection;
@@ -76,41 +75,71 @@ abstract class Relation implements BuilderContract
7675
*/
7776
protected static $selfJoinCount = 0;
7877

78+
protected static ?string $noConstraintsForRelationName = null;
79+
7980
/**
8081
* Create a new relation instance.
8182
*
8283
* @param \Illuminate\Database\Eloquent\Builder $query
8384
* @param \Illuminate\Database\Eloquent\Model $parent
85+
* @param \Illuminate\Database\Eloquent\Model|null $resourceModel
8486
* @return void
8587
*/
86-
public function __construct(Builder $query, Model $parent)
88+
public function __construct(Builder $query, Model $parent, ?Model $resourceModel = null)
8789
{
8890
$this->query = $query;
8991
$this->parent = $parent;
9092
$this->related = $query->getModel();
93+
$resourceModel ??= $parent;
94+
95+
if (
96+
'' !== (string)static::$noConstraintsForRelationName
97+
|| '' !== (string)$resourceModel->nowEagerLoadingRelationNameWithNoConstraints
98+
) {
99+
/**
100+
* 1st execution is for ExampleModel $exampleModel on 'rel' relation
101+
* with nowEagerLoadingRelationNameWithNoConstraints = 'rel'
102+
* and with $noConstraintsForRelationName = 'rel'
103+
*/
104+
/* 2nd execution is for UserModel $userModel on 'categories' relation
105+
with nowEagerLoadingRelationNameWithNoConstraints = null
106+
and with $noConstraintsForRelationName = 'rel' */
107+
108+
109+
/* 1st execution is for ExampleModel $exampleModel on 'children' relation
110+
with nowEagerLoadingRelationNameWithNoConstraints = null
111+
and with $noConstraintsForRelationName = 'rel' */
112+
/**
113+
* 2nd execution is for ExampleModel $exampleModel on 'rel' relation
114+
* with nowEagerLoadingRelationNameWithNoConstraints = 'rel'
115+
* and with $noConstraintsForRelationName = 'rel'
116+
*/
117+
/* 3rd execution is for UserModel $userModel on 'categories' relation
118+
with nowEagerLoadingRelationNameWithNoConstraints = null
119+
and with $noConstraintsForRelationName = 'rel' */
120+
static::$constraints =
121+
static::$noConstraintsForRelationName !== $resourceModel->nowEagerLoadingRelationNameWithNoConstraints;
122+
}
91123

92124
$this->addConstraints();
93125
}
94126

95127
/**
96128
* Run a callback with constraints disabled on the relation.
97129
*
98-
* @param \Closure $callback
99130
* @return mixed
100131
*/
101-
public static function noConstraints(Closure $callback)
132+
public static function noConstraints(\Closure $callback, string $relationName)
102133
{
103134
$previous = static::$constraints;
135+
$previousNoConstraintsForRelationName = static::$noConstraintsForRelationName;
136+
static::$noConstraintsForRelationName = $relationName;
104137

105-
static::$constraints = false;
106-
107-
// When resetting the relation where clause, we want to shift the first element
108-
// off of the bindings, leaving only the constraints that the developers put
109-
// as "extra" on the relationships, and not original relation constraints.
110138
try {
111139
return $callback();
112140
} finally {
113141
static::$constraints = $previous;
142+
static::$noConstraintsForRelationName = $previousNoConstraintsForRelationName;
114143
}
115144
}
116145

0 commit comments

Comments
 (0)