CVE-2011-4183: Unrestricted Upload of File with Dangerous Type (CWE-434)
value: A vulnerability in open build service allows remote attackers to upload arbitrary RPM files. Affected releases are SUSE open build service prior to 2.1.16.
CWE description: The product allows the upload or transfer of dangerous file types that are automatically processed within its environment.
CVSS v3 base score: 9.8
Published: 2018-06-13T13:29Z
Repository: https://github.com/openSUSE/open-build-service
Language: Ruby
Files changed: src/api/app/controllers/build_controller.rb, src/api/test/functional/build_controller_test.rb
Commit date: 2011-12-12 15:51:03 +0100
Commit message: [api] support binary package upload for the admin

Diff: diff --git a/src/api/app/controllers/build_controller.rb b/src/api/app/controllers/build_controller.rb
index 20b4ef73c..0ad4a5e1d 100644
--- a/src/api/app/controllers/build_controller.rb
+++ b/src/api/app/controllers/build_controller.rb
@@ -1,18 +1,30 @@
 class BuildController < ApplicationController
 
   def index
-    valid_http_methods :get, :post, :put
+    valid_http_methods :get, :post
 
     # for permission check
     if params[:package] and not ["_repository", "_jobhistory"].include?(params[:package])
       pkg = DbPackage.get_by_project_and_name( params[:project], params[:package], use_source=false )
     else
       prj = DbProject.get_by_name params[:project]
     end
 
-    pass_to_backend 
+    if request.get?
+      pass_to_backend 
+      return
+    end
+
+    if @http_user.is_admin?
+      # check for a local package instance
+      DbPackage.get_by_project_and_name( params[:project], params[:package], follow_project_links=false )
+      pass_to_backend
+    else
+      render_error :status => 403, :errorcode => "execute_cmd_no_permission",
+        :message => "Upload of binaries is only permitted for administrators"
+    end
   end
 
   def project_index
     valid_http_methods :get, :post, :put
 
@@ -126,25 +138,10 @@ class BuildController < ApplicationController
     DbProject.get_by_name params[:project]
 
     pass_to_backend
   end
 
-  # /build/:prj/:repo/:arch/:pkg
-  def package_index
-    valid_http_methods :get
-    required_parameters :project, :repository, :arch, :package
-
-    # read access permission check
-    if params[:package] == "_repository"
-      prj = DbProject.get_by_name params[:project], use_source=false
-    else
-      pkg = DbPackage.get_by_project_and_name params[:project], params[:package], use_source=false
-    end
-
-    pass_to_backend
-  end
-
   # /build/:project/:repository/:arch/:package/:filename
   def file
     valid_http_methods :get, :delete
     required_parameters :project, :repository, :arch, :package, :filename
 
diff --git a/src/api/test/functional/build_controller_test.rb b/src/api/test/functional/build_controller_test.rb
index 8b113ecc1..8b8d2b961 100644
--- a/src/api/test/functional/build_controller_test.rb
+++ b/src/api/test/functional/build_controller_test.rb
@@ -37,10 +37,36 @@ class BuildControllerTest < ActionController::IntegrationTest
     assert_response 404
     get "/build/home:Iggy/10.2/blabla"
     assert_response 404
   end
 
+  def test_upload_binaries
+    ActionController::IntegrationTest::reset_auth
+    post "/build/home:Iggy/10.2/i586/TestPack", nil
+    assert_response 401
+
+    prepare_request_with_user "adrian", "so_alone"
+    post "/build/home:Iggy/10.2/i586/TestPack", nil
+    assert_response 403
+
+    prepare_request_with_user "king", "sunflower"
+    post "/build/home:Iggy/10.2/i586/TestPack", nil
+    assert_response 400 # actually a success, it reached the backend
+    assert_tag :tag => "status", :attributes => { :code => "400", :origin => "backend" }
+
+    # check not supported methods
+    post "/build/home:Iggy/10.2/i586/_repository", nil
+    assert_response 404
+    assert_tag :tag => "status", :attributes => { :code => "unknown_package" }
+    put "/build/home:Iggy/10.2/i586/TestPack", nil
+    assert_response 400
+    assert_tag :tag => "status", :attributes => { :code => "invalid_http_method" }
+    delete "/build/home:Iggy/10.2/i586/TestPack"
+    assert_response 400
+    assert_tag :tag => "status", :attributes => { :code => "invalid_http_method" }
+  end
+
   def test_dispatchprios
     ActionController::IntegrationTest::reset_auth
     get "/build/_dispatchprios"
     assert_response 401
 

Vulnerable code:     valid_http_methods :get, :post, :put
    pass_to_backend 
  # /build/:prj/:repo/:arch/:pkg
  def package_index
    valid_http_methods :get
    required_parameters :project, :repository, :arch, :package

    # read access permission check
    if params[:package] == "_repository"
      prj = DbProject.get_by_name params[:project], use_source=false
    else
      pkg = DbPackage.get_by_project_and_name params[:project], params[:package], use_source=false
    end

    pass_to_backend
  end

Fixed code:     valid_http_methods :get, :post
    if request.get?
      pass_to_backend 
      return
    end

    if @http_user.is_admin?
      # check for a local package instance
      DbPackage.get_by_project_and_name( params[:project], params[:package], follow_project_links=false )
      pass_to_backend
    else
      render_error :status => 403, :errorcode => "execute_cmd_no_permission",
        :message => "Upload of binaries is only permitted for administrators"
    end
  def test_upload_binaries
    ActionController::IntegrationTest::reset_auth
    post "/build/home:Iggy/10.2/i586/TestPack", nil
    assert_response 401

    prepare_request_with_user "adrian", "so_alone"
    post "/build/home:Iggy/10.2/i586/TestPack", nil
    assert_response 403

    prepare_request_with_user "king", "sunflower"
    post "/build/home:Iggy/10.2/i586/TestPack", nil
    assert_response 400 # actually a success, it reached the backend
    assert_tag :tag => "status", :attributes => { :code => "400", :origin => "backend" }

    # check not supported methods
    post "/build/home:Iggy/10.2/i586/_repository", nil
    assert_response 404
    assert_tag :tag => "status", :attributes => { :code => "unknown_package" }
    put "/build/home:Iggy/10.2/i586/TestPack", nil
    assert_response 400
    assert_tag :tag => "status", :attributes => { :code => "invalid_http_method" }
    delete "/build/home:Iggy/10.2/i586/TestPack"
    assert_response 400
    assert_tag :tag => "status", :attributes => { :code => "invalid_http_method" }
  end