Skip to content
This repository
Browse code

[api][backend] fix handling of deleted projects, this is closing anot…

…her shipstopper security leak for 2.2. Michael, please review backend part.
  • Loading branch information...
commit a209085bd9a46f85d419ecd031a644b77df52f0e 1 parent ccea7af
Adrian Schröter authored October 31, 2011
24  src/api/app/controllers/source_controller.rb
@@ -83,7 +83,7 @@ def index_project
83 83
     #---------------------
84 84
     if request.get?
85 85
       if params.has_key? :deleted
86  
-        # FIXME2.2: this would grants access to hidden projects
  86
+        validate_visibility_of_deleted_project(project_name)
87 87
         pass_to_backend
88 88
       else
89 89
         if DbProject.is_remote_project? project_name
@@ -2208,6 +2208,26 @@ def validate_read_access_of_deleted_package(project, name)
2208 2208
     raise DbPackage::UnknownObjectError, "#{project}/#{name}" unless r
2209 2209
     dpkg = Package.new(r.body)
2210 2210
     raise DbPackage::UnknownObjectError, "#{project}/#{name}" unless dpkg
2211  
-    raise DbPackage::ReadSourceAccessError, "#{project}/#{name}" if dpkg.disabled_for? 'sourceaccess'
  2211
+    raise DbPackage::ReadSourceAccessError, "#{project}/#{name}" if dpkg.disabled_for? 'sourceaccess' and not @http_user.is_admin?
2212 2212
   end
  2213
+
  2214
+  def validate_visibility_of_deleted_project(project)
  2215
+    begin
  2216
+      r = Suse::Backend.get("/source/#{CGI.escape(project)}/_project/_history?deleted=1&meta=1")
  2217
+    rescue
  2218
+      raise DbProject::UnknownObjectError, "#{project}"
  2219
+    end
  2220
+
  2221
+    data = ActiveXML::XMLNode.new(r.body.to_s)
  2222
+    lastrev = nil
  2223
+    data.each_revision {|rev| lastrev = rev}
  2224
+    raise DbProject::UnknownObjectError, "#{project}" unless lastrev
  2225
+
  2226
+    metapath = "/source/#{CGI.escape(project)}/_project/_meta?rev=#{lastrev.value('srcmd5')}&deleted=1"
  2227
+    r = Suse::Backend.get(metapath)
  2228
+    dprj = Project.new(r.body)
  2229
+    #FIXME: actually a per user checking would be more accurate here
  2230
+    raise DbProject::UnknownObjectError, "#{project}" if dprj.nil? or (dprj.disabled_for? 'access' and not @http_user.is_admin?)
  2231
+  end
  2232
+
2213 2233
 end
11  src/api/test/functional/read_permission_test.rb
@@ -646,7 +646,7 @@ def test_project_links_to_sourceaccess_protected_project
646 646
   end
647 647
 
648 648
   def test_project_links_to_read_access_protected_projects
649  
-    # Create public project with protected package
  649
+    # Create public project with sourceaccess protected package
650 650
     prepare_request_with_user "tom", "thunder"
651 651
 
652 652
     # try to link to an access protected hidden project from sourceaccess project
@@ -718,6 +718,15 @@ def test_project_links_to_read_access_protected_projects
718 718
     assert_response :success
719 719
     delete "/source/home:adrian:ProtectedProject4"
720 720
     assert_response :success
  721
+
  722
+    # validate handling of deleted project
  723
+    get "/source/home:adrian:ProtectedProject4"
  724
+    assert_response 404
  725
+    get "/source/home:adrian:ProtectedProject4?deleted=1"
  726
+    assert_response :success
  727
+    prepare_request_with_user "tom", "thunder"
  728
+    get "/source/home:adrian:ProtectedProject4?deleted=1"
  729
+    assert_response 404
721 730
   end
722 731
 
723 732
   def test_compare_error_messages
25  src/backend/bs_srcserver
@@ -2863,19 +2863,19 @@ sub undeleteproject {
2863 2863
 
2864 2864
 sub getpackagelist {
2865 2865
   my ($cgi, $projid, $repoid, $arch) = @_;
2866  
-  my $proj = checkprojrepoarch($projid, $repoid, $arch, 1);
2867  
-  if ($proj->{'remoteurl'}) {
2868  
-    my @args;
2869  
-    push @args, "deleted" if $cgi->{'deleted'};
2870  
-    return BSRPC::rpc({'uri' => "$proj->{'remoteurl'}/source/$proj->{'remoteproject'}", 'proxy' => $proxy}, $BSXML::dir, @args), $BSXML::dir;
2871  
-  }
2872 2866
   my @packages;
2873  
-  if (!$cgi->{'deleted'}) {
2874  
-    @packages = findpackages($projid, $proj);
2875  
-  } else {
  2867
+  if ($cgi->{'deleted'}) {
2876 2868
     @packages = grep {s/\.mrev\.del$//} ls("$projectsdir/$projid.pkg");
2877 2869
     @packages = grep {! -e "$projectsdir/$projid.pkg/$_.xml"} @packages;
2878 2870
     @packages = sort @packages;
  2871
+  } else {
  2872
+    my $proj = checkprojrepoarch($projid, $repoid, $arch, 1);
  2873
+    if ($proj->{'remoteurl'}) {
  2874
+      my @args;
  2875
+      push @args, "deleted" if $cgi->{'deleted'};
  2876
+      return BSRPC::rpc({'uri' => "$proj->{'remoteurl'}/source/$proj->{'remoteproject'}", 'proxy' => $proxy}, $BSXML::dir, @args), $BSXML::dir;
  2877
+    }
  2878
+    @packages = findpackages($projid, $proj);
2879 2879
   }
2880 2880
   my @plist = map {{'name' => $_}} @packages;
2881 2881
   return ({'entry' => \@plist}, $BSXML::dir);
@@ -2883,8 +2883,9 @@ sub getpackagelist {
2883 2883
 
2884 2884
 sub getpackage {
2885 2885
   my ($cgi, $projid, $packid) = @_;
2886  
-  my $proj = checkprojrepoarch($projid, undef, undef, 1);
2887  
-  if ($proj->{'remoteurl'}) {
  2886
+  my $proj;
  2887
+  $proj = checkprojrepoarch($projid, undef, undef, 1) unless $cgi->{'deleted'};
  2888
+  if ($proj && $proj->{'remoteurl'}) {
2888 2889
     my @args;
2889 2890
     push @args, "rev=$cgi->{'rev'}" if $cgi->{'rev'};
2890 2891
     my $pack = BSRPC::rpc({'uri' => "$proj->{'remoteurl'}/source/$proj->{'remoteproject'}/$packid/_meta", 'proxy' => $proxy}, $BSXML::pack, @args);
@@ -6436,7 +6437,7 @@ my $dispatches = [
6436 6437
   '/source/$project/$package view=info rev? linkrev? parse:bool? repository? arch?' => \&getpackagesourceinfo,
6437 6438
   '/source/$project/$package rev? linkrev? emptylink:bool? expand:bool? view:? extension:? lastworking:bool? withlinked:bool? meta:bool?' => \&getfilelist,
6438 6439
   '/source/$project/$package/_history rev? meta:bool? deleted:bool? limit:num?' => \&getpackagehistory,
6439  
-  '/source/$project/$package/_meta rev?' => \&getpackage,
  6440
+  '/source/$project/$package/_meta rev? deleted:bool?' => \&getpackage,
6440 6441
   'PUT:/source/$project/$package/_meta user:? comment:? requestid:num?' => \&putpackage,
6441 6442
   'DELETE:/source/$project/$package user:? comment:? requestid:num?' => \&delpackage,
6442 6443
   '/source/$project/$package/$filename rev? expand:bool? meta:bool?' => \&getfile,

0 notes on commit a209085

Please sign in to comment.
Something went wrong with that request. Please try again.