◐ Shell
reader mode source ↗
Skip to content

Commit 15ff74a

Browse files
jdomeracki-coderf0ssel
jdomeracki-coder
and
f0ssel
authored
fix: validate FileSize in NewDataBuilder to prevent OOM DoS (#25710) (#26185)
Backport of #25710 to `release/2.33`. Cherry-picked with `git cherry-pick -x` (`a2e1ddb56f`); the commit body references the original PR. _Generated by Coder Agents on behalf of @jdomeracki-coder._ Co-authored-by: Garrett Delfosse <garrett@coder.com>
1 parent 97f9e3d commit 15ff74a

7 files changed

Lines changed: 161 additions & 24 deletions

File tree

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1609,7 +1609,10 @@ func (s *server) DownloadFile(request *proto.FileRequest, stream proto.DRPCProvi
16091609
return fail(xerrors.Errorf("unsupported file upload type: %s", request.UploadType))
16101610
}
16111611

1612-
upload, chunks := sdkproto.BytesToDataUpload(sdkproto.DataUploadType_UPLOAD_TYPE_MODULE_FILES, file.Data)
1612+
upload, chunks, err := sdkproto.BytesToDataUpload(sdkproto.DataUploadType_UPLOAD_TYPE_MODULE_FILES, file.Data)
1613+
if err != nil {
1614+
return fail(xerrors.Errorf("prepare file upload: %w", err))
1615+
}
16131616

16141617
err = stream.Send(&sdkproto.FileUpload{
16151618
Type: &sdkproto.FileUpload_DataUpload{DataUpload: upload},
Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,8 @@ func TestUploadFileLargeModuleFiles(t *testing.T) {
4848
require.NoError(t, err)
4949

5050
// Convert to upload format
51-
upload, chunks := sdkproto.BytesToDataUpload(sdkproto.DataUploadType_UPLOAD_TYPE_MODULE_FILES, moduleData)
51+
upload, chunks, err := sdkproto.BytesToDataUpload(sdkproto.DataUploadType_UPLOAD_TYPE_MODULE_FILES, moduleData)
52+
require.NoError(t, err)
5253

5354
stream := newMockUploadStream(upload, chunks...)
5455

@@ -93,7 +94,8 @@ func TestUploadFileErrorScenarios(t *testing.T) {
9394
_, err := crand.Read(moduleData)
9495
require.NoError(t, err)
9596

96-
upload, chunks := sdkproto.BytesToDataUpload(sdkproto.DataUploadType_UPLOAD_TYPE_MODULE_FILES, moduleData)
97+
upload, chunks, err := sdkproto.BytesToDataUpload(sdkproto.DataUploadType_UPLOAD_TYPE_MODULE_FILES, moduleData)
98+
require.NoError(t, err)
9799

98100
t.Run("chunk_before_upload", func(t *testing.T) {
99101
t.Parallel()
Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -533,7 +533,10 @@ func (p *Server) UploadModuleFiles(ctx context.Context, moduleFiles []byte) erro
533533
}
534534
defer stream.Close()
535535

536-
dataUp, chunks := sdkproto.BytesToDataUpload(sdkproto.DataUploadType_UPLOAD_TYPE_MODULE_FILES, moduleFiles)
536+
dataUp, chunks, err := sdkproto.BytesToDataUpload(sdkproto.DataUploadType_UPLOAD_TYPE_MODULE_FILES, moduleFiles)
537+
if err != nil {
538+
return nil, xerrors.Errorf("prepare module files upload: %w", err)
539+
}
537540

538541
err = stream.Send(&sdkproto.FileUpload{Type: &sdkproto.FileUpload_DataUpload{DataUpload: dataUp}})
539542
if err != nil {
Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,17 @@ func (r *Runner) init(ctx context.Context, omitModules bool, templateArchive []b
1919
// If `moduleTar` is populated, `init` will send it over in multiple parts. This
2020
// It must be called before the initial request to populate the correct hash if
2121
// there is data to send. This is safe to call on nil or empty slices.
22-
data, chunks := sdkproto.BytesToDataUpload(sdkproto.DataUploadType_UPLOAD_TYPE_MODULE_FILES, moduleTar)
22+
data, chunks, err := sdkproto.BytesToDataUpload(sdkproto.DataUploadType_UPLOAD_TYPE_MODULE_FILES, moduleTar)
23+
if err != nil {
24+
return nil, r.failedJobf("prepare module files upload: %v", err)
25+
}
2326

2427
hash := []byte{}
2528
if len(moduleTar) > 0 {
2629
hash = data.DataHash
2730
}
2831

29-
err := r.session.Send(&sdkproto.Request{Type: &sdkproto.Request_Init{Init: &sdkproto.InitRequest{
32+
err = r.session.Send(&sdkproto.Request{Type: &sdkproto.Request_Init{Init: &sdkproto.InitRequest{
3033
TemplateSourceArchive: templateArchive,
3134
OmitModuleFiles: omitModules,
3235
InitialModuleTarHash: hash,
Lines changed: 24 additions & 4 deletions
Load DiffSome generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 104 additions & 2 deletions
Load DiffSome generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -246,24 +246,28 @@ func (s *Session) handleInitRequest(init *proto.InitRequest, requests <-chan *pr
246246
s.Logger.Info(s.Context(), "plan response too large, sending modules as stream",
247247
slog.F("size_bytes", len(complete.ModuleFiles)),
248248
)
249-
dataUp, chunks := proto.BytesToDataUpload(proto.DataUploadType_UPLOAD_TYPE_MODULE_FILES, complete.ModuleFiles)
250-
251-
complete.ModuleFiles = nil // sent over the stream
252-
complete.ModuleFilesHash = dataUp.DataHash
253-
254-
err := s.stream.Send(&proto.Response{Type: &proto.Response_DataUpload{DataUpload: dataUp}})
249+
dataUp, chunks, err := proto.BytesToDataUpload(proto.DataUploadType_UPLOAD_TYPE_MODULE_FILES, complete.ModuleFiles)
255250
if err != nil {
256-
complete.Error = fmt.Sprintf("send data upload: %s", err.Error())
251+
complete.Error = fmt.Sprintf("prepare module files upload: %s", err.Error())
257252
} else {
258-
for i, chunk := range chunks {
259-
err := s.stream.Send(&proto.Response{Type: &proto.Response_ChunkPiece{ChunkPiece: chunk}})
260-
if err != nil {
261-
complete.Error = fmt.Sprintf("send data piece upload %d/%d: %s", i, dataUp.Chunks, err.Error())
262-
break
253+
complete.ModuleFiles = nil // sent over the stream
254+
complete.ModuleFilesHash = dataUp.DataHash
255+
256+
err := s.stream.Send(&proto.Response{Type: &proto.Response_DataUpload{DataUpload: dataUp}})
257+
if err != nil {
258+
complete.Error = fmt.Sprintf("send data upload: %s", err.Error())
259+
} else {
260+
for i, chunk := range chunks {
261+
err := s.stream.Send(&proto.Response{Type: &proto.Response_ChunkPiece{ChunkPiece: chunk}})
262+
if err != nil {
263+
complete.Error = fmt.Sprintf("send data piece upload %d/%d: %s", i, dataUp.Chunks, err.Error())
264+
break
265+
}
263266
}
264267
}
265268
}
266269
}
270+
267271
s.initialized = true
268272

269273
return complete, nil

0 commit comments

Comments
 (0)